From cdd7ffc847d8feb09bcafd2357928dd78a109853 Mon Sep 17 00:00:00 2001 From: Michael Tughan Date: Fri, 21 Feb 2025 10:20:50 -0500 Subject: [PATCH 1/2] Run Spotless Apply --- pom.xml | 35 +++++----- .../plugins/gitserver/CSRFExclusionImpl.java | 39 ++++++----- .../plugins/gitserver/ChannelTransport.java | 51 +++++++------- .../FileBackedHttpGitRepository.java | 31 ++++----- .../plugins/gitserver/HttpGitRepository.java | 67 +++++++++---------- .../plugins/gitserver/RepositoryResolver.java | 3 +- .../gitserver/ssh/AbstractGitCommand.java | 19 +++--- .../gitserver/ssh/ReceivePackCommand.java | 7 +- .../gitserver/ssh/SshCommandFactoryImpl.java | 8 +-- .../gitserver/ssh/UploadPackCommand.java | 8 +-- .../gitserver/ssh/ReceivePackCommandTest.java | 25 ++++--- .../gitserver/ssh/Security3342Test.java | 40 ++++++----- 12 files changed, 170 insertions(+), 163 deletions(-) diff --git a/pom.xml b/pom.xml index d5c263a..c684dfa 100644 --- a/pom.xml +++ b/pom.xml @@ -1,3 +1,4 @@ + 4.0.0 @@ -5,7 +6,7 @@ org.jenkins-ci.plugins plugin 4.86 - + git-server @@ -15,21 +16,6 @@ Jenkins Git server Plugin https://github.com/jenkinsci/${project.artifactId}-plugin - - 999999-SNAPSHOT - - 2.440 - ${jenkins.baseline}.3 - jenkinsci/${project.artifactId}-plugin - - - - scm:git:https://github.com/${gitHubRepo}.git - scm:git:git@github.com:${gitHubRepo}.git - https://github.com/${gitHubRepo} - ${scmTag} - - The MIT license @@ -38,14 +24,29 @@ + + scm:git:https://github.com/${gitHubRepo}.git + scm:git:git@github.com:${gitHubRepo}.git + ${scmTag} + https://github.com/${gitHubRepo} + + + + 999999-SNAPSHOT + + 2.440 + ${jenkins.baseline}.3 + jenkinsci/${project.artifactId}-plugin + + io.jenkins.tools.bom bom-${jenkins.baseline}.x 3234.v5ca_5154341ef - import pom + import diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java b/src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java index d946724..b158ba3 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java @@ -2,7 +2,11 @@ import hudson.Extension; import hudson.security.csrf.CrumbExclusion; - +import java.io.IOException; +import java.util.Collections; +import java.util.Enumeration; +import java.util.Map; +import java.util.Vector; import javax.servlet.FilterChain; import javax.servlet.ReadListener; import javax.servlet.ServletException; @@ -10,15 +14,10 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.util.Collections; -import java.util.Enumeration; -import java.util.Map; -import java.util.Vector; /** * CSRF exclusion for git-upload-pack. - * + * *

* We do some basic checks to significantly limit the scope of exclusion, but * because of the dynamic nature of the URL structure, this doesn't guarantee @@ -26,7 +25,7 @@ * * So to further protect Jenkins, we pass through a fake {@link HttpServletRequest} * that masks the values of the submission. - * + * *

* If the fake request is routed to {@link HttpGitRepository}, which is * the only legitimate destination of the request, we'll unwrap this fake request @@ -35,19 +34,19 @@ *

* In this way, even if an attacker manages to route the request to elsewhere in Jenkins, * that request will not be interpreted as a POST request. - * + * * @author Kohsuke Kawaguchi */ @Extension public class CSRFExclusionImpl extends CrumbExclusion { - public boolean process(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { - if (!"application/x-git-receive-pack-request".equals(request.getHeader("Content-Type"))) - return false; + public boolean process(HttpServletRequest request, HttpServletResponse response, FilterChain chain) + throws IOException, ServletException { + if (!"application/x-git-receive-pack-request".equals(request.getHeader("Content-Type"))) return false; -// String path = request.getPathInfo(); -// if(!path.contains("/repo.git/") || !path.endsWith("/git-receive-pack")) -// return false; + // String path = request.getPathInfo(); + // if(!path.contains("/repo.git/") || !path.endsWith("/git-receive-pack")) + // return false; HttpServletRequestWrapper w = new HttpServletRequestWrapper(request) { @Override @@ -72,7 +71,7 @@ public Enumeration getParameterNames() { @Override public String[] getParameterValues(String name) { - return new String[]{"bogus"}; + return new String[] {"bogus"}; } @Override @@ -105,13 +104,13 @@ public void setReadListener(ReadListener readListener) { }; } }; - w.setAttribute(ORIGINAL_REQUEST,request); - - chain.doFilter(w,response); + w.setAttribute(ORIGINAL_REQUEST, request); + + chain.doFilter(w, response); return true; } - static final String ORIGINAL_REQUEST = CSRFExclusionImpl.class.getName()+".originalRequest"; + static final String ORIGINAL_REQUEST = CSRFExclusionImpl.class.getName() + ".originalRequest"; public static HttpServletRequest unwrapRequest(HttpServletRequest req) { return (HttpServletRequest) req.getAttribute(CSRFExclusionImpl.ORIGINAL_REQUEST); diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java b/src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java index 06d82e7..67c6b9d 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java @@ -1,9 +1,13 @@ package org.jenkinsci.plugins.gitserver; import hudson.FilePath; -import hudson.FilePath.FileCallable; import hudson.remoting.Pipe; import hudson.remoting.VirtualChannel; +import java.io.BufferedInputStream; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.IOException; +import java.net.URISyntaxException; import jenkins.MasterToSlaveFileCallable; import org.apache.commons.io.IOUtils; import org.eclipse.jgit.errors.NotSupportedException; @@ -20,12 +24,6 @@ import org.eclipse.jgit.transport.URIish; import org.eclipse.jgit.transport.UploadPack; -import java.io.BufferedInputStream; -import java.io.BufferedOutputStream; -import java.io.File; -import java.io.IOException; -import java.net.URISyntaxException; - /** * {@link Transport} implementation across pipes. * @@ -34,15 +32,14 @@ public class ChannelTransport extends Transport implements PackTransport { private final FilePath remoteRepository; - public static Transport open(Repository local, FilePath remoteRepository) throws NotSupportedException, URISyntaxException, TransportException { - if (remoteRepository.isRemote()) - return new ChannelTransport(local,remoteRepository); - else - return Transport.open(local,remoteRepository.getRemote()); + public static Transport open(Repository local, FilePath remoteRepository) + throws NotSupportedException, URISyntaxException, TransportException { + if (remoteRepository.isRemote()) return new ChannelTransport(local, remoteRepository); + else return Transport.open(local, remoteRepository.getRemote()); } public ChannelTransport(Repository local, FilePath remoteRepository) throws URISyntaxException { - super(local, new URIish("channel:"+remoteRepository.getRemote())); + super(local, new URIish("channel:" + remoteRepository.getRemote())); this.remoteRepository = remoteRepository; } @@ -54,15 +51,17 @@ public FetchConnection openFetch() throws NotSupportedException, TransportExcept try { remoteRepository.actAsync(new GitFetchTask(l2r, r2l)); } catch (IOException e) { - throw new TransportException("Failed to open a fetch connection",e); + throw new TransportException("Failed to open a fetch connection", e); } catch (InterruptedException e) { - throw new TransportException("Failed to open a fetch connection",e); + throw new TransportException("Failed to open a fetch connection", e); } - return new BasePackFetchConnection(this) {{ - init(new BufferedInputStream(r2l.getIn()), new BufferedOutputStream(l2r.getOut())); - readAdvertisedRefs(); - }}; + return new BasePackFetchConnection(this) { + { + init(new BufferedInputStream(r2l.getIn()), new BufferedOutputStream(l2r.getOut())); + readAdvertisedRefs(); + } + }; } @Override @@ -73,15 +72,17 @@ public PushConnection openPush() throws NotSupportedException, TransportExceptio try { remoteRepository.actAsync(new GitPushTask(l2r, r2l)); } catch (IOException e) { - throw new TransportException("Failed to open a fetch connection",e); + throw new TransportException("Failed to open a fetch connection", e); } catch (InterruptedException e) { - throw new TransportException("Failed to open a fetch connection",e); + throw new TransportException("Failed to open a fetch connection", e); } - return new BasePackPushConnection(this) {{ - init(new BufferedInputStream(r2l.getIn()), new BufferedOutputStream(l2r.getOut())); - readAdvertisedRefs(); - }}; + return new BasePackPushConnection(this) { + { + init(new BufferedInputStream(r2l.getIn()), new BufferedOutputStream(l2r.getOut())); + readAdvertisedRefs(); + } + }; } @Override diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java b/src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java index ea64f95..ca669d0 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java @@ -1,5 +1,13 @@ package org.jenkinsci.plugins.gitserver; +import java.io.File; +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.Collection; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.servlet.http.HttpServletRequest; import jenkins.model.Jenkins; import org.acegisecurity.Authentication; import org.eclipse.jgit.api.AddCommand; @@ -18,15 +26,6 @@ import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; -import javax.servlet.http.HttpServletRequest; -import java.io.File; -import java.io.IOException; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.util.Collection; -import java.util.logging.Level; -import java.util.logging.Logger; - /** * Convenient subtype of {@link HttpGitRepository} where the repository * is non-bare, resides in a directory local to the controller, and you maintain @@ -76,11 +75,11 @@ protected void createInitialRepository(Repository r) throws IOException { cmd.call(); CommitCommand co = git.commit(); - co.setAuthor("Jenkins","noreply@jenkins-ci.org"); + co.setAuthor("Jenkins", "noreply@jenkins-ci.org"); co.setMessage("Initial import of the existing contents"); co.call(); } catch (GitAPIException e) { - LOGGER.log(Level.WARNING, "Initial import of "+workspace+" into Git repository failed",e); + LOGGER.log(Level.WARNING, "Initial import of " + workspace + " into Git repository failed", e); } } @@ -94,7 +93,8 @@ protected void createInitialRepository(Repository r) throws IOException { * where /foo/bar is rwx------ and /foo/bar/zot is rwxrwxrwx.) */ @Override - public UploadPack createUploadPack(HttpServletRequest context, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException { + public UploadPack createUploadPack(HttpServletRequest context, Repository db) + throws ServiceNotEnabledException, ServiceNotAuthorizedException { return new UploadPack(db); } @@ -102,12 +102,13 @@ public UploadPack createUploadPack(HttpServletRequest context, Repository db) th * Requires the admin access to be able to push */ @Override - public ReceivePack createReceivePack(HttpServletRequest context, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException { + public ReceivePack createReceivePack(HttpServletRequest context, Repository db) + throws ServiceNotEnabledException, ServiceNotAuthorizedException { Authentication a = Jenkins.getAuthentication(); ReceivePack rp = createReceivePack(db); - rp.setRefLogIdent(new PersonIdent(a.getName(), a.getName()+"@"+context.getRemoteAddr())); + rp.setRefLogIdent(new PersonIdent(a.getName(), a.getName() + "@" + context.getRemoteAddr())); return rp; } @@ -125,7 +126,7 @@ public void onPostReceive(ReceivePack rp, Collection commands) { } catch (Exception e) { StringWriter sw = new StringWriter(); e.printStackTrace(new PrintWriter(sw)); - rp.sendMessage("Failed to update workspace: "+sw); + rp.sendMessage("Failed to update workspace: " + sw); } } }); diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java b/src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java index 9c050a7..4434e19 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java @@ -1,6 +1,15 @@ package org.jenkinsci.plugins.gitserver; import hudson.model.Action; +import java.io.IOException; +import java.util.Enumeration; +import java.util.Vector; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.servlet.ServletConfig; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; import jenkins.model.Jenkins; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.http.server.GitServlet; @@ -15,16 +24,6 @@ import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; -import javax.servlet.ServletConfig; -import javax.servlet.ServletContext; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import java.io.IOException; -import java.util.Enumeration; -import java.util.Vector; -import java.util.logging.Level; -import java.util.logging.Logger; - /** * UI-bound object that exposes a Git repository via HTTP. * @@ -38,8 +37,7 @@ public abstract class HttpGitRepository { private GitServlet g; private Exception causeOfDeath; - public HttpGitRepository() { - } + public HttpGitRepository() {} /** * Opens the repository this UI-bound object holds on to. @@ -60,7 +58,8 @@ public HttpGitRepository() { * * @see ReceivePackFactory#create(Object, Repository) */ - public abstract ReceivePack createReceivePack(HttpServletRequest context, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException; + public abstract ReceivePack createReceivePack(HttpServletRequest context, Repository db) + throws ServiceNotEnabledException, ServiceNotAuthorizedException; /** * Returns the {@link UploadPack} that handles "git fetch" from client. @@ -73,9 +72,10 @@ public HttpGitRepository() { * * @see UploadPackFactory#create(Object, Repository) */ - public abstract UploadPack createUploadPack(HttpServletRequest context, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException; + public abstract UploadPack createUploadPack(HttpServletRequest context, Repository db) + throws ServiceNotEnabledException, ServiceNotAuthorizedException; - /** + /** * to make sure the user has the permission to pull. */ public void checkPullPermission() { @@ -89,21 +89,23 @@ public Repository open(HttpServletRequest req, String name) throws RepositoryNot try { return openRepository(); } catch (IOException e) { - throw new RepositoryNotFoundException(req.getRequestURI(),e); + throw new RepositoryNotFoundException(req.getRequestURI(), e); } } }); // this creates (and thus configures) the receiver program g.setReceivePackFactory(new ReceivePackFactory() { - public ReceivePack create(HttpServletRequest req, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException { - return createReceivePack(req,db); + public ReceivePack create(HttpServletRequest req, Repository db) + throws ServiceNotEnabledException, ServiceNotAuthorizedException { + return createReceivePack(req, db); } }); g.setUploadPackFactory(new UploadPackFactory() { - public UploadPack create(HttpServletRequest req, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException { - return createUploadPack(req,db); + public UploadPack create(HttpServletRequest req, Repository db) + throws ServiceNotEnabledException, ServiceNotAuthorizedException { + return createUploadPack(req, db); } }); @@ -130,7 +132,7 @@ public Enumeration getInitParameterNames() { } }); } catch (ServletException e) { - LOGGER.log(Level.WARNING,"Failed to initialize GitServlet for " + this,e); + LOGGER.log(Level.WARNING, "Failed to initialize GitServlet for " + this, e); causeOfDeath = e; } return g; @@ -140,27 +142,24 @@ public Enumeration getInitParameterNames() { * Handles git smart HTTP protocol. */ public void doDynamic(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException { - if (g==null) - g=init(); + if (g == null) g = init(); - if (causeOfDeath!=null) - throw new ServletException(causeOfDeath); + if (causeOfDeath != null) throw new ServletException(causeOfDeath); // This is one place where we allow POST without CSRF headers HttpServletRequest realRequest = CSRFExclusionImpl.unwrapRequest(req); - if (realRequest==null) - realRequest = req; + if (realRequest == null) realRequest = req; /* - GitServlet uses getPathInfo() to determine the current request, whereas in Stapler's sense it should be using - getRestOfPath(). + GitServlet uses getPathInfo() to determine the current request, whereas in Stapler's sense it should be using + getRestOfPath(). - However, this nicely cancels out with the the GitServlet behavior of wanting one token that specifies - the repository to work with. + However, this nicely cancels out with the the GitServlet behavior of wanting one token that specifies + the repository to work with. - So in this case one bug cancels out another and it works out well. - */ - g.service(realRequest,rsp); + So in this case one bug cancels out another and it works out well. + */ + g.service(realRequest, rsp); } private static final Logger LOGGER = Logger.getLogger(HttpGitRepository.class.getName()); diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/RepositoryResolver.java b/src/main/java/org/jenkinsci/plugins/gitserver/RepositoryResolver.java index 7485f8a..dd9cef9 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/RepositoryResolver.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/RepositoryResolver.java @@ -2,13 +2,12 @@ import hudson.ExtensionList; import hudson.ExtensionPoint; +import java.io.IOException; import jenkins.model.Jenkins; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.UploadPack; -import java.io.IOException; - /** * Resolves the full name of the repository as given by Git clients to actual {@link Repository}. * diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/AbstractGitCommand.java b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/AbstractGitCommand.java index bcb05f2..f549055 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/AbstractGitCommand.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/AbstractGitCommand.java @@ -15,7 +15,7 @@ * @author Kohsuke Kawaguchi */ abstract class AbstractGitCommand extends AsynchronousCommand { - @Argument(index=0, metaVar="REPO", required=true, usage="repository name") + @Argument(index = 0, metaVar = "REPO", required = true, usage = "repository name") protected String repoName; AbstractGitCommand(CommandLine cmdLine) { @@ -24,15 +24,16 @@ abstract class AbstractGitCommand extends AsynchronousCommand { @Override protected final int runCommand() throws Exception { - try { - ParserProperties properties = ParserProperties.defaults().withAtSyntax(false); - new CmdLineParser(this, properties).parseArgument(getCmdLine().subList(1,getCmdLine().size())); - } catch (CmdLineException e) { - throw new AbortException(e.getMessage()); - } + try { + ParserProperties properties = ParserProperties.defaults().withAtSyntax(false); + new CmdLineParser(this, properties) + .parseArgument(getCmdLine().subList(1, getCmdLine().size())); + } catch (CmdLineException e) { + throw new AbortException(e.getMessage()); + } - return doRun(); + return doRun(); } - + protected abstract int doRun() throws Exception; } diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommand.java b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommand.java index 386ee59..68e983e 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommand.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommand.java @@ -19,13 +19,12 @@ public ReceivePackCommand(CommandLine cmdLine) { protected int doRun() throws Exception { for (RepositoryResolver rr : RepositoryResolver.all()) { ReceivePack rp = rr.createReceivePack(repoName); - if (rp!=null) { - rp.receive(getInputStream(),getOutputStream(),getErrorStream()); + if (rp != null) { + rp.receive(getInputStream(), getOutputStream(), getErrorStream()); return 0; } } - throw new AbortException("No such repository exists:"+repoName); - + throw new AbortException("No such repository exists:" + repoName); } } diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/SshCommandFactoryImpl.java b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/SshCommandFactoryImpl.java index 59368e4..4189d6d 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/SshCommandFactoryImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/SshCommandFactoryImpl.java @@ -11,12 +11,10 @@ public class SshCommandFactoryImpl extends SshCommandFactory { @Override public Command create(CommandLine cmds) { - if (cmds.size()<1) return null; + if (cmds.size() < 1) return null; - if (cmds.get(0).equals("git-receive-pack")) - return new ReceivePackCommand(cmds); - if (cmds.get(0).equals("git-upload-pack")) - return new UploadPackCommand(cmds); + if (cmds.get(0).equals("git-receive-pack")) return new ReceivePackCommand(cmds); + if (cmds.get(0).equals("git-upload-pack")) return new UploadPackCommand(cmds); return null; } diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/UploadPackCommand.java b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/UploadPackCommand.java index a91be13..e882094 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/UploadPackCommand.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/UploadPackCommand.java @@ -20,12 +20,12 @@ public UploadPackCommand(CommandLine cmdLine) { protected int doRun() throws Exception { for (RepositoryResolver rr : RepositoryResolver.all()) { UploadPack up = rr.createUploadPack(repoName); - if (up!=null) { - up.upload(getInputStream(),getOutputStream(),getErrorStream()); + if (up != null) { + up.upload(getInputStream(), getOutputStream(), getErrorStream()); return 0; } } - throw new AbortException("No such repository exists:"+repoName); + throw new AbortException("No such repository exists:" + repoName); } -} \ No newline at end of file +} diff --git a/src/test/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommandTest.java b/src/test/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommandTest.java index 6d26c9b..1c0675a 100644 --- a/src/test/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommandTest.java +++ b/src/test/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommandTest.java @@ -1,10 +1,22 @@ package org.jenkinsci.plugins.gitserver.ssh; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; + import hudson.Functions; import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.net.InetSocketAddress; import java.nio.file.Files; import java.nio.file.Path; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; import java.util.EnumSet; +import java.util.concurrent.TimeUnit; import org.apache.sshd.client.SshClient; import org.apache.sshd.client.channel.ClientChannel; import org.apache.sshd.client.channel.ClientChannelEvent; @@ -20,19 +32,6 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; -import java.io.IOException; -import java.net.InetSocketAddress; -import java.security.KeyPair; -import java.security.KeyPairGenerator; -import java.security.NoSuchAlgorithmException; -import java.util.concurrent.TimeUnit; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeFalse; - /** * This class contains code borrowed and adapted from the Jenkins SSHD plugin. * Original source: org.jenkinsci.main.modules.sshd.SSHDTest.java diff --git a/src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java b/src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java index 6cd6b66..65c53be 100644 --- a/src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java +++ b/src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java @@ -1,7 +1,16 @@ package org.jenkinsci.plugins.gitserver.ssh; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeFalse; + import hudson.Functions; +import java.io.File; +import java.io.IOException; import java.net.InetSocketAddress; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; import java.security.PublicKey; import java.util.Collections; import java.util.List; @@ -23,16 +32,6 @@ import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.MockAuthorizationStrategy; -import java.io.File; -import java.io.IOException; -import java.security.KeyPair; -import java.security.KeyPairGenerator; -import java.security.NoSuchAlgorithmException; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.junit.Assume.assumeFalse; - public class Security3342Test { @Rule @@ -51,7 +50,8 @@ public void setUp() { public void openRepositoryPermissionCheckTest() throws Exception { j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().to("tester")); + j.jenkins.setAuthorizationStrategy( + new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().to("tester")); hudson.model.User tester = hudson.model.User.getOrCreateByIdOrFullName("tester"); KeyPair keyPair = generateKeys(tester); j.jenkins.save(); @@ -70,12 +70,18 @@ protected Iterable getDefaultKeys(File sshDir) { protected ServerKeyDatabase getServerKeyDatabase(File homeDir, File sshDir) { return new ServerKeyDatabase() { @Override - public List lookup(String connectAddress, InetSocketAddress remoteAddress, Configuration config) { + public List lookup( + String connectAddress, InetSocketAddress remoteAddress, Configuration config) { return Collections.emptyList(); } @Override - public boolean accept(String connectAddress, InetSocketAddress remoteAddress, PublicKey serverKey, Configuration config, CredentialsProvider provider) { + public boolean accept( + String connectAddress, + InetSocketAddress remoteAddress, + PublicKey serverKey, + Configuration config, + CredentialsProvider provider) { return true; } }; @@ -104,12 +110,16 @@ public boolean accept(String connectAddress, InetSocketAddress remoteAddress, Pu Git gitClone2 = clone.call(); } catch (Exception e) { // Verify that the expected exception was caught with the correct message - assertTrue(e.getCause() != null && e.getCause().getMessage().contains("hudson.security.AccessDeniedException3: tester is missing the Overall/Read permission")); + assertTrue( + e.getCause() != null + && e.getCause() + .getMessage() + .contains( + "hudson.security.AccessDeniedException3: tester is missing the Overall/Read permission")); } // Verify that the .git directory is not created File gitDir2 = new File(dir2, ".git"); assertFalse(".git directory exist, clone operation succeed", gitDir2.exists()); - } private static KeyPair generateKeys(hudson.model.User user) throws NoSuchAlgorithmException, IOException { From 238480ba2519311ef90b1bf43cc42fed89ab31a1 Mon Sep 17 00:00:00 2001 From: Michael Tughan Date: Thu, 21 Nov 2024 09:59:39 -0500 Subject: [PATCH 2/2] Modernize to Jenkins 2.479 and Jakarta EE 9 * Adapter methods are added for old overrides. * Switch to JUnit 5 for tests --- .gitignore | 1 + pom.xml | 10 +- .../plugins/gitserver/CSRFExclusionImpl.java | 41 +++--- .../plugins/gitserver/ChannelTransport.java | 10 +- .../FileBackedHttpGitRepository.java | 51 +++++--- .../plugins/gitserver/HttpGitRepository.java | 122 ++++++++++++------ .../plugins/gitserver/RepositoryResolver.java | 9 +- .../gitserver/ssh/ReceivePackCommand.java | 3 +- .../gitserver/ssh/SshCommandFactoryImpl.java | 2 +- .../gitserver/ssh/UploadPackCommand.java | 7 +- .../gitserver/ssh/ReceivePackCommandTest.java | 38 ++---- .../gitserver/ssh/Security3342Test.java | 89 ++++++------- 12 files changed, 207 insertions(+), 176 deletions(-) diff --git a/.gitignore b/.gitignore index 3e41e15..491e58d 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ mvn.out* .settings/ .repository/ .settings.xml +.idea/ diff --git a/pom.xml b/pom.xml index c684dfa..1e448ea 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.jenkins-ci.plugins plugin - 4.86 + 5.6 @@ -34,8 +34,8 @@ 999999-SNAPSHOT - 2.440 - ${jenkins.baseline}.3 + 2.479 + ${jenkins.baseline}.1 jenkinsci/${project.artifactId}-plugin @@ -44,7 +44,7 @@ io.jenkins.tools.bom bom-${jenkins.baseline}.x - 3234.v5ca_5154341ef + 4023.va_eeb_b_4e45f07 pom import @@ -68,7 +68,7 @@ org.eclipse.jgit org.eclipse.jgit.ssh.apache - 6.9.0.202403050737-r + 7.0.0.202409031743-r test diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java b/src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java index b158ba3..f56036a 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/CSRFExclusionImpl.java @@ -2,18 +2,18 @@ import hudson.Extension; import hudson.security.csrf.CrumbExclusion; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletInputStream; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.Collections; import java.util.Enumeration; +import java.util.Locale; import java.util.Map; -import java.util.Vector; -import javax.servlet.FilterChain; -import javax.servlet.ReadListener; -import javax.servlet.ServletException; -import javax.servlet.ServletInputStream; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletRequestWrapper; -import javax.servlet.http.HttpServletResponse; /** * CSRF exclusion for git-upload-pack. @@ -23,6 +23,7 @@ * because of the dynamic nature of the URL structure, this doesn't guarantee * that we have no leak. * + *

* So to further protect Jenkins, we pass through a fake {@link HttpServletRequest} * that masks the values of the submission. * @@ -39,48 +40,46 @@ */ @Extension public class CSRFExclusionImpl extends CrumbExclusion { + private static final String BOGUS = "bogus"; + @Override public boolean process(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { if (!"application/x-git-receive-pack-request".equals(request.getHeader("Content-Type"))) return false; - // String path = request.getPathInfo(); - // if(!path.contains("/repo.git/") || !path.endsWith("/git-receive-pack")) - // return false; - HttpServletRequestWrapper w = new HttpServletRequestWrapper(request) { @Override public String getQueryString() { - return "bogus"; + return BOGUS; } @Override public String getParameter(String name) { - return "bogus"; + return BOGUS; } @Override - public Map getParameterMap() { + public Map getParameterMap() { return Collections.emptyMap(); } @Override - public Enumeration getParameterNames() { - return new Vector().elements(); + public Enumeration getParameterNames() { + return Collections.emptyEnumeration(); } @Override public String[] getParameterValues(String name) { - return new String[] {"bogus"}; + return new String[] {BOGUS}; } @Override public String getMethod() { - return "BOGUS"; + return BOGUS.toUpperCase(Locale.ROOT); } @Override - public ServletInputStream getInputStream() throws IOException { + public ServletInputStream getInputStream() { return new ServletInputStream() { @Override public boolean isFinished() { @@ -93,7 +92,7 @@ public boolean isReady() { } @Override - public int read() throws IOException { + public int read() { return -1; } diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java b/src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java index 67c6b9d..e48216e 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/ChannelTransport.java @@ -53,6 +53,7 @@ public FetchConnection openFetch() throws NotSupportedException, TransportExcept } catch (IOException e) { throw new TransportException("Failed to open a fetch connection", e); } catch (InterruptedException e) { + Thread.currentThread().interrupt(); throw new TransportException("Failed to open a fetch connection", e); } @@ -74,6 +75,7 @@ public PushConnection openPush() throws NotSupportedException, TransportExceptio } catch (IOException e) { throw new TransportException("Failed to open a fetch connection", e); } catch (InterruptedException e) { + Thread.currentThread().interrupt(); throw new TransportException("Failed to open a fetch connection", e); } @@ -100,15 +102,13 @@ public GitFetchTask(Pipe l2r, Pipe r2l) { } public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { - Repository repo = new FileRepositoryBuilder().setWorkTree(f).build(); - try { + try (Repository repo = new FileRepositoryBuilder().setWorkTree(f).build()) { final UploadPack rp = new UploadPack(repo); rp.upload(new BufferedInputStream(l2r.getIn()), new BufferedOutputStream(r2l.getOut()), null); return null; } finally { IOUtils.closeQuietly(l2r.getIn()); IOUtils.closeQuietly(r2l.getOut()); - repo.close(); } } } @@ -123,15 +123,13 @@ public GitPushTask(Pipe l2r, Pipe r2l) { } public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { - Repository repo = new FileRepositoryBuilder().setWorkTree(f).build(); - try { + try (Repository repo = new FileRepositoryBuilder().setWorkTree(f).build()) { final ReceivePack rp = new ReceivePack(repo); rp.receive(new BufferedInputStream(l2r.getIn()), new BufferedOutputStream(r2l.getOut()), null); return null; } finally { IOUtils.closeQuietly(l2r.getIn()); IOUtils.closeQuietly(r2l.getOut()); - repo.close(); } } } diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java b/src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java index ca669d0..09202d8 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/FileBackedHttpGitRepository.java @@ -1,15 +1,16 @@ package org.jenkinsci.plugins.gitserver; +import jakarta.servlet.http.HttpServletRequest; import java.io.File; import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.Collection; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.logging.Level; import java.util.logging.Logger; -import javax.servlet.http.HttpServletRequest; import jenkins.model.Jenkins; -import org.acegisecurity.Authentication; import org.eclipse.jgit.api.AddCommand; import org.eclipse.jgit.api.CommitCommand; import org.eclipse.jgit.api.Git; @@ -19,12 +20,11 @@ import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; -import org.eclipse.jgit.transport.PostReceiveHook; -import org.eclipse.jgit.transport.ReceiveCommand; import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.UploadPack; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; +import org.springframework.security.core.Authentication; /** * Convenient subtype of {@link HttpGitRepository} where the repository @@ -38,19 +38,28 @@ public abstract class FileBackedHttpGitRepository extends HttpGitRepository { * Directory of the local workspace on the controller. * There will be "./.git" that hosts the actual repository. */ - public final File workspace; + public final Path workspace; - protected FileBackedHttpGitRepository(File workspace) { + protected FileBackedHttpGitRepository(Path workspace) { this.workspace = workspace; - if (!workspace.exists() && !workspace.mkdirs()) { - LOGGER.log(Level.WARNING, "Cannot create a workspace in {0}", workspace); + try { + Files.createDirectory(workspace); + } catch (FileAlreadyExistsException ignored) { + // don't need to worry about this; if it already exists, we don't care! + } catch (IOException e) { + LOGGER.log(Level.WARNING, e, () -> "Cannot create a workspace in " + workspace); } } + protected FileBackedHttpGitRepository(File workspace) { + this(workspace.toPath()); + } + @Override public Repository openRepository() throws IOException { checkPullPermission(); - Repository r = new FileRepositoryBuilder().setWorkTree(workspace).build(); + Repository r = + new FileRepositoryBuilder().setWorkTree(workspace.toFile()).build(); // if the repository doesn't exist, create it if (!r.getObjectDatabase().exists()) { @@ -62,6 +71,7 @@ public Repository openRepository() throws IOException { /** * Called when there's no .git directory to create one. * + *

* This implementation also imports whatever currently in there into the repository. */ protected void createInitialRepository(Repository r) throws IOException { @@ -79,7 +89,7 @@ protected void createInitialRepository(Repository r) throws IOException { co.setMessage("Initial import of the existing contents"); co.call(); } catch (GitAPIException e) { - LOGGER.log(Level.WARNING, "Initial import of " + workspace + " into Git repository failed", e); + LOGGER.log(Level.WARNING, e, () -> "Initial import of " + workspace + " into Git repository failed"); } } @@ -87,6 +97,7 @@ protected void createInitialRepository(Repository r) throws IOException { * This default implementation allows read access to anyone * who can access the HTTP URL this repository is bound to. * + *

* For example, if this object is used as a project action, * and the project isn't readable to Alice, then Alice won't be * able to pull from this repository (think of a POSIX file system @@ -104,7 +115,7 @@ public UploadPack createUploadPack(HttpServletRequest context, Repository db) @Override public ReceivePack createReceivePack(HttpServletRequest context, Repository db) throws ServiceNotEnabledException, ServiceNotAuthorizedException { - Authentication a = Jenkins.getAuthentication(); + Authentication a = Jenkins.getAuthentication2(); ReceivePack rp = createReceivePack(db); @@ -119,15 +130,13 @@ public ReceivePack createReceivePack(Repository db) { ReceivePack rp = new ReceivePack(db); // update userContent after the push - rp.setPostReceiveHook(new PostReceiveHook() { - public void onPostReceive(ReceivePack rp, Collection commands) { - try { - updateWorkspace(rp.getRepository()); - } catch (Exception e) { - StringWriter sw = new StringWriter(); - e.printStackTrace(new PrintWriter(sw)); - rp.sendMessage("Failed to update workspace: " + sw); - } + rp.setPostReceiveHook((rp1, commands) -> { + try { + updateWorkspace(rp1.getRepository()); + } catch (Exception e) { + StringWriter sw = new StringWriter(); + e.printStackTrace(new PrintWriter(sw)); + rp1.sendMessage("Failed to update workspace: " + sw); } }); return rp; diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java b/src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java index 4434e19..0c6d01f 100644 --- a/src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java +++ b/src/main/java/org/jenkinsci/plugins/gitserver/HttpGitRepository.java @@ -1,15 +1,17 @@ package org.jenkinsci.plugins.gitserver; +import hudson.Util; import hudson.model.Action; +import io.jenkins.servlet.http.HttpServletRequestWrapper; +import jakarta.servlet.ServletConfig; +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; +import java.util.Collections; import java.util.Enumeration; -import java.util.Vector; import java.util.logging.Level; import java.util.logging.Logger; -import javax.servlet.ServletConfig; -import javax.servlet.ServletContext; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; import jenkins.model.Jenkins; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.http.server.GitServlet; @@ -21,8 +23,8 @@ import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.eclipse.jgit.transport.resolver.UploadPackFactory; -import org.kohsuke.stapler.StaplerRequest; -import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.StaplerRequest2; +import org.kohsuke.stapler.StaplerResponse2; /** * UI-bound object that exposes a Git repository via HTTP. @@ -37,7 +39,7 @@ public abstract class HttpGitRepository { private GitServlet g; private Exception causeOfDeath; - public HttpGitRepository() {} + protected HttpGitRepository() {} /** * Opens the repository this UI-bound object holds on to. @@ -47,6 +49,7 @@ public HttpGitRepository() {} /** * Returns the {@link ReceivePack} that handles "git push" from client. * + *

* The most basic implementation is the following, which allows anyone to push to this repository, * so normally you want some kind of access check before that. {@link DefaultReceivePackFactory} isn't suitable * here because it requires that the user has non-empty name, which isn't necessarily true in Jenkins @@ -58,12 +61,39 @@ public HttpGitRepository() {} * * @see ReceivePackFactory#create(Object, Repository) */ - public abstract ReceivePack createReceivePack(HttpServletRequest context, Repository db) - throws ServiceNotEnabledException, ServiceNotAuthorizedException; + @SuppressWarnings({"deprecated", "java:S1874"}) + public ReceivePack createReceivePack(HttpServletRequest context, Repository db) + throws ServiceNotEnabledException, ServiceNotAuthorizedException { + if (Util.isOverridden( + HttpGitRepository.class, + getClass(), + "createReceivePack", + javax.servlet.http.HttpServletRequest.class, + Repository.class)) { + return createReceivePack(HttpServletRequestWrapper.fromJakartaHttpServletRequest(context), db); + } + throw new AbstractMethodError("Implementing class '" + this.getClass().getName() + "' does not override " + + "either overload of the createReceivePack method."); + } + + /** + * @deprecated Override {@link #createReceivePack(HttpServletRequest, Repository)} instead. + */ + @Deprecated(since = "134") + public ReceivePack createReceivePack(javax.servlet.http.HttpServletRequest context, Repository db) + throws ServiceNotEnabledException, ServiceNotAuthorizedException { + if (Util.isOverridden( + HttpGitRepository.class, getClass(), "createReceivePack", HttpServletRequest.class, Repository.class)) { + return createReceivePack(HttpServletRequestWrapper.toJakartaHttpServletRequest(context), db); + } + throw new AbstractMethodError("Implementing class '" + this.getClass().getName() + "' does not override " + + "either overload of the createReceivePack method."); + } /** * Returns the {@link UploadPack} that handles "git fetch" from client. * + *

* The most basic implementation is the following, which exposes this repository to everyone. * * 

@@ -72,42 +102,56 @@ public abstract ReceivePack createReceivePack(HttpServletRequest context, Reposi
      *
      * @see UploadPackFactory#create(Object, Repository)
      */
-    public abstract UploadPack createUploadPack(HttpServletRequest context, Repository db)
-            throws ServiceNotEnabledException, ServiceNotAuthorizedException;
+    @SuppressWarnings({"deprecated", "java:S1874"})
+    public UploadPack createUploadPack(HttpServletRequest context, Repository db)
+            throws ServiceNotEnabledException, ServiceNotAuthorizedException {
+        if (Util.isOverridden(
+                HttpGitRepository.class,
+                getClass(),
+                "createUploadPack",
+                javax.servlet.http.HttpServletRequest.class,
+                Repository.class)) {
+            return createUploadPack(HttpServletRequestWrapper.fromJakartaHttpServletRequest(context), db);
+        }
+        throw new AbstractMethodError("Implementing class '" + this.getClass().getName() + "' does not override "
+                + "either overload of the createUploadPack method.");
+    }
+
+    /**
+     * @deprecated Override {@link #createUploadPack(HttpServletRequest, Repository)} instead.
+     */
+    @Deprecated(since = "134")
+    public UploadPack createUploadPack(javax.servlet.http.HttpServletRequest context, Repository db)
+            throws ServiceNotEnabledException, ServiceNotAuthorizedException {
+        if (Util.isOverridden(
+                HttpGitRepository.class, getClass(), "createUploadPack", HttpServletRequest.class, Repository.class)) {
+            return createUploadPack(HttpServletRequestWrapper.toJakartaHttpServletRequest(context), db);
+        }
+        throw new AbstractMethodError("Implementing class '" + this.getClass().getName() + "' does not override "
+                + "either overload of the createUploadPack method.");
+    }
 
     /**
      * to make sure the user has the permission to pull.
      */
     public void checkPullPermission() {
-        Jenkins.getInstance().checkPermission(Jenkins.READ);
+        Jenkins.get().checkPermission(Jenkins.READ);
     }
 
     protected GitServlet init() {
         GitServlet g = new GitServlet();
-        g.setRepositoryResolver(new org.eclipse.jgit.transport.resolver.RepositoryResolver() {
-            public Repository open(HttpServletRequest req, String name) throws RepositoryNotFoundException {
-                try {
-                    return openRepository();
-                } catch (IOException e) {
-                    throw new RepositoryNotFoundException(req.getRequestURI(), e);
-                }
+        g.setRepositoryResolver((req, name) -> {
+            try {
+                return openRepository();
+            } catch (IOException e) {
+                throw new RepositoryNotFoundException(req.getRequestURI(), e);
             }
         });
 
         // this creates (and thus configures) the receiver program
-        g.setReceivePackFactory(new ReceivePackFactory() {
-            public ReceivePack create(HttpServletRequest req, Repository db)
-                    throws ServiceNotEnabledException, ServiceNotAuthorizedException {
-                return createReceivePack(req, db);
-            }
-        });
+        g.setReceivePackFactory(this::createReceivePack);
 
-        g.setUploadPackFactory(new UploadPackFactory() {
-            public UploadPack create(HttpServletRequest req, Repository db)
-                    throws ServiceNotEnabledException, ServiceNotAuthorizedException {
-                return createUploadPack(req, db);
-            }
-        });
+        g.setUploadPackFactory(this::createUploadPack);
 
         try {
             g.init(new ServletConfig() {
@@ -116,23 +160,19 @@ public String getServletName() {
                 }
 
                 public ServletContext getServletContext() throws IllegalStateException {
-                    Jenkins j = Jenkins.getInstance();
-                    if (j == null) {
-                        throw new IllegalStateException();
-                    }
-                    return j.servletContext;
+                    return Jenkins.get().getServletContext();
                 }
 
                 public String getInitParameter(String name) {
                     return null;
                 }
 
-                public Enumeration getInitParameterNames() {
-                    return new Vector().elements();
+                public Enumeration getInitParameterNames() {
+                    return Collections.emptyEnumeration();
                 }
             });
         } catch (ServletException e) {
-            LOGGER.log(Level.WARNING, "Failed to initialize GitServlet for " + this, e);
+            LOGGER.log(Level.WARNING, e, () -> "Failed to initialize GitServlet for " + this);
             causeOfDeath = e;
         }
         return g;
@@ -141,7 +181,7 @@ public Enumeration getInitParameterNames() {
     /**
      * Handles git smart HTTP protocol.
      */
-    public void doDynamic(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
+    public void doDynamic(StaplerRequest2 req, StaplerResponse2 rsp) throws IOException, ServletException {
         if (g == null) g = init();
 
         if (causeOfDeath != null) throw new ServletException(causeOfDeath);
diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/RepositoryResolver.java b/src/main/java/org/jenkinsci/plugins/gitserver/RepositoryResolver.java
index dd9cef9..728cc47 100644
--- a/src/main/java/org/jenkinsci/plugins/gitserver/RepositoryResolver.java
+++ b/src/main/java/org/jenkinsci/plugins/gitserver/RepositoryResolver.java
@@ -1,5 +1,6 @@
 package org.jenkinsci.plugins.gitserver;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
 import hudson.ExtensionList;
 import hudson.ExtensionPoint;
 import java.io.IOException;
@@ -47,6 +48,7 @@ public abstract class RepositoryResolver implements ExtensionPoint {
      *      null if this resolver doesn't recognize the given path name.
      *      This will allow other {@link RepositoryResolver}s to get a shot at the repository.
      */
+    @CheckForNull
     public abstract ReceivePack createReceivePack(String fullRepositoryName) throws IOException, InterruptedException;
 
     /**
@@ -60,13 +62,10 @@ public abstract class RepositoryResolver implements ExtensionPoint {
      *      null if this resolver doesn't recognize the given path name.
      *      This will allow other {@link RepositoryResolver}s to get a shot at the repository.
      */
+    @CheckForNull
     public abstract UploadPack createUploadPack(String fullRepositoryName) throws IOException, InterruptedException;
 
     public static ExtensionList all() throws IllegalStateException {
-        Jenkins j = Jenkins.getInstance();
-        if (j == null) {
-            throw new IllegalStateException();
-        }
-        return j.getExtensionList(RepositoryResolver.class);
+        return Jenkins.get().getExtensionList(RepositoryResolver.class);
     }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommand.java b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommand.java
index 68e983e..839a2ea 100644
--- a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommand.java
+++ b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommand.java
@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.gitserver.ssh;
 
 import hudson.AbortException;
+import java.io.IOException;
 import org.eclipse.jgit.transport.ReceivePack;
 import org.jenkinsci.main.modules.sshd.SshCommandFactory.CommandLine;
 import org.jenkinsci.plugins.gitserver.RepositoryResolver;
@@ -16,7 +17,7 @@ public ReceivePackCommand(CommandLine cmdLine) {
     }
 
     @Override
-    protected int doRun() throws Exception {
+    protected int doRun() throws IOException, InterruptedException {
         for (RepositoryResolver rr : RepositoryResolver.all()) {
             ReceivePack rp = rr.createReceivePack(repoName);
             if (rp != null) {
diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/SshCommandFactoryImpl.java b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/SshCommandFactoryImpl.java
index 4189d6d..da2ab39 100644
--- a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/SshCommandFactoryImpl.java
+++ b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/SshCommandFactoryImpl.java
@@ -11,7 +11,7 @@
 public class SshCommandFactoryImpl extends SshCommandFactory {
     @Override
     public Command create(CommandLine cmds) {
-        if (cmds.size() < 1) return null;
+        if (cmds.isEmpty()) return null;
 
         if (cmds.get(0).equals("git-receive-pack")) return new ReceivePackCommand(cmds);
         if (cmds.get(0).equals("git-upload-pack")) return new UploadPackCommand(cmds);
diff --git a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/UploadPackCommand.java b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/UploadPackCommand.java
index e882094..1ddb04f 100644
--- a/src/main/java/org/jenkinsci/plugins/gitserver/ssh/UploadPackCommand.java
+++ b/src/main/java/org/jenkinsci/plugins/gitserver/ssh/UploadPackCommand.java
@@ -1,6 +1,7 @@
 package org.jenkinsci.plugins.gitserver.ssh;
 
 import hudson.AbortException;
+import java.io.IOException;
 import org.eclipse.jgit.transport.UploadPack;
 import org.jenkinsci.main.modules.sshd.SshCommandFactory.CommandLine;
 import org.jenkinsci.plugins.gitserver.RepositoryResolver;
@@ -17,11 +18,13 @@ public UploadPackCommand(CommandLine cmdLine) {
     }
 
     @Override
-    protected int doRun() throws Exception {
+    protected int doRun() throws IOException, InterruptedException {
         for (RepositoryResolver rr : RepositoryResolver.all()) {
             UploadPack up = rr.createUploadPack(repoName);
             if (up != null) {
-                up.upload(getInputStream(), getOutputStream(), getErrorStream());
+                try (up) {
+                    up.upload(getInputStream(), getOutputStream(), getErrorStream());
+                }
                 return 0;
             }
         }
diff --git a/src/test/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommandTest.java b/src/test/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommandTest.java
index 1c0675a..826a1fa 100644
--- a/src/test/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommandTest.java
+++ b/src/test/java/org/jenkinsci/plugins/gitserver/ssh/ReceivePackCommandTest.java
@@ -1,12 +1,9 @@
 package org.jenkinsci.plugins.gitserver.ssh;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.not;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeFalse;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import hudson.Functions;
+import hudson.model.User;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.net.InetSocketAddress;
@@ -26,30 +23,23 @@
 import org.jenkinsci.main.modules.cli.auth.ssh.PublicKeySignatureWriter;
 import org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl;
 import org.jenkinsci.main.modules.sshd.SSHD;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
 
 /**
  * This class contains code borrowed and adapted from the Jenkins SSHD plugin.
  * Original source: org.jenkinsci.main.modules.sshd.SSHDTest.java
  */
-public class ReceivePackCommandTest {
-
-    @Rule
-    public JenkinsRule j = new JenkinsRule();
-
-    @Before
-    public void setUp() {
-        assumeFalse(Functions.isWindows());
-    }
+@WithJenkins
+class ReceivePackCommandTest {
+    private static final String USER = "enabled";
 
     @Test
     @Issue("SECURITY-3319")
-    public void shouldNotParseAtChar() throws Exception {
-        hudson.model.User enabled = hudson.model.User.getOrCreateByIdOrFullName("enabled");
+    void shouldNotParseAtChar(JenkinsRule ignored) throws Exception {
+        User enabled = User.getOrCreateByIdOrFullName(USER);
         KeyPair keyPair = generateKeys(enabled);
         SSHD server = SSHD.get();
         server.setPort(0);
@@ -63,7 +53,7 @@ public void shouldNotParseAtChar() throws Exception {
         try (SshClient client = SshClient.setUpDefaultClient()) {
             client.setServerKeyVerifier(AcceptAllServerKeyVerifier.INSTANCE);
             client.start();
-            ConnectFuture future = client.connect("enabled", new InetSocketAddress(server.getActualPort()));
+            ConnectFuture future = client.connect(USER, new InetSocketAddress("localhost", server.getActualPort()));
             try (ClientSession session = future.verify(10, TimeUnit.SECONDS).getSession()) {
                 session.addPublicKeyIdentity(keyPair);
                 assertTrue(session.auth().await(10, TimeUnit.SECONDS));
@@ -76,14 +66,14 @@ public void shouldNotParseAtChar() throws Exception {
                     channel.waitFor(EnumSet.of(ClientChannelEvent.CLOSED), TimeUnit.SECONDS.toMillis(5));
 
                     String errorMessage = errorStream.toString();
-                    assertThat(errorMessage, containsString("@" + tempPath));
-                    assertThat(errorMessage, not(containsString(content)));
+                    assertTrue(errorMessage.contains("@" + tempPath));
+                    assertFalse(errorMessage.contains(content));
                 }
             }
         }
     }
 
-    private static KeyPair generateKeys(hudson.model.User user) throws NoSuchAlgorithmException, IOException {
+    private static KeyPair generateKeys(User user) throws NoSuchAlgorithmException, IOException {
         // I'd prefer to generate Ed25519 keys here, but the API is too awkward currently
         // ECDSA keys would be even more awkward as we'd need a copy of the curve parameters
         KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA");
diff --git a/src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java b/src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java
index 65c53be..2dbec2b 100644
--- a/src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java
+++ b/src/test/java/org/jenkinsci/plugins/gitserver/ssh/Security3342Test.java
@@ -1,22 +1,27 @@
 package org.jenkinsci.plugins.gitserver.ssh;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeFalse;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import hudson.Functions;
+import hudson.model.User;
 import java.io.File;
 import java.io.IOException;
 import java.net.InetSocketAddress;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.NoSuchAlgorithmException;
 import java.security.PublicKey;
-import java.util.Collections;
 import java.util.List;
 import jenkins.model.Jenkins;
 import org.eclipse.jgit.api.CloneCommand;
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.InvalidRemoteException;
+import org.eclipse.jgit.errors.NoRemoteRepositoryException;
 import org.eclipse.jgit.transport.CredentialsProvider;
 import org.eclipse.jgit.transport.SshSessionFactory;
 import org.eclipse.jgit.transport.sshd.ServerKeyDatabase;
@@ -24,41 +29,31 @@
 import org.jenkinsci.main.modules.cli.auth.ssh.PublicKeySignatureWriter;
 import org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl;
 import org.jenkinsci.main.modules.sshd.SSHD;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.MockAuthorizationStrategy;
+import org.jvnet.hudson.test.junit.jupiter.WithJenkins;
 
-public class Security3342Test {
-
-    @Rule
-    public JenkinsRule j = new JenkinsRule();
-
-    @Rule
-    public TemporaryFolder tmp = new TemporaryFolder();
-
-    @Before
-    public void setUp() {
-        assumeFalse(Functions.isWindows());
-    }
+@WithJenkins
+class Security3342Test {
+    private static final String USER = "tester";
 
     @Test
     @Issue("SECURITY-3342")
-    public void openRepositoryPermissionCheckTest() throws Exception {
+    void openRepositoryPermissionCheckTest(@TempDir Path tmp, JenkinsRule j) throws Exception {
 
         j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
         j.jenkins.setAuthorizationStrategy(
-                new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().to("tester"));
-        hudson.model.User tester = hudson.model.User.getOrCreateByIdOrFullName("tester");
+                new MockAuthorizationStrategy().grant(Jenkins.READ).everywhere().to(USER));
+        User tester = User.getOrCreateByIdOrFullName(USER);
         KeyPair keyPair = generateKeys(tester);
         j.jenkins.save();
 
         // Fixed ssh port for Jenkins ssh server
         SSHD server = SSHD.get();
-        server.setPort(2222);
+        server.setPort(0);
 
         final SshdSessionFactory instance = new SshdSessionFactory() {
             @Override
@@ -72,7 +67,7 @@ protected ServerKeyDatabase getServerKeyDatabase(File homeDir, File sshDir) {
                     @Override
                     public List lookup(
                             String connectAddress, InetSocketAddress remoteAddress, Configuration config) {
-                        return Collections.emptyList();
+                        return List.of();
                     }
 
                     @Override
@@ -90,39 +85,35 @@ public boolean accept(
         SshSessionFactory.setInstance(instance);
 
         CloneCommand clone = Git.cloneRepository();
-        clone.setURI("ssh://tester@localhost:2222/userContent.git");
-        File dir1 = tmp.newFolder();
-        clone.setDirectory(dir1);
+        clone.setURI("ssh://" + USER + "@localhost:" + server.getActualPort() + "/userContent.git");
+        Path dir1 = Files.createTempDirectory(tmp, null);
+        clone.setDirectory(dir1.toFile());
 
         // Do the git clone for a user with Jenkins.READ permission
-        Git gitClone1 = clone.call();
-        File gitDir1 = new File(dir1, ".git");
-        assertTrue(".git directory exist, clone operation succeed", gitDir1.exists());
+        assertDoesNotThrow(clone::call).close();
+        Path gitDir1 = dir1.resolve(".git");
+        assertTrue(Files.isDirectory(gitDir1), ".git directory exist, clone operation succeed");
 
         // Do the git clone for a user without Jenkins.READ permission
         j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy());
         j.jenkins.save();
 
-        File dir2 = tmp.newFolder();
-        clone.setDirectory(dir2);
-
-        try {
-            Git gitClone2 = clone.call();
-        } catch (Exception e) {
-            // Verify that the expected exception was caught with the correct message
-            assertTrue(
-                    e.getCause() != null
-                            && e.getCause()
-                                    .getMessage()
-                                    .contains(
-                                            "hudson.security.AccessDeniedException3: tester is missing the Overall/Read permission"));
-        }
+        Path dir2 = Files.createTempDirectory(tmp, null);
+        clone.setDirectory(dir2.toFile());
+
+        // Verify that the expected exception was caught with the correct message
+        InvalidRemoteException e = assertThrows(InvalidRemoteException.class, clone::call);
+        assertInstanceOf(NoRemoteRepositoryException.class, e.getCause());
+        NoRemoteRepositoryException e2 = (NoRemoteRepositoryException) e.getCause();
+        assertTrue(e2.getMessage()
+                .contains("hudson.security.AccessDeniedException3: tester is missing the Overall/Read permission"));
+
         // Verify that the .git directory is not created
-        File gitDir2 = new File(dir2, ".git");
-        assertFalse(".git directory exist, clone operation succeed", gitDir2.exists());
+        Path gitDir2 = dir2.resolve(".git");
+        assertFalse(Files.isDirectory(gitDir2), ".git directory exist, clone operation succeed");
     }
 
-    private static KeyPair generateKeys(hudson.model.User user) throws NoSuchAlgorithmException, IOException {
+    private static KeyPair generateKeys(User user) throws NoSuchAlgorithmException, IOException {
         // I'd prefer to generate Ed25519 keys here, but the API is too awkward currently
         // ECDSA keys would be even more awkward as we'd need a copy of the curve parameters
         KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA");