Multiple vulnerabilities allow bypassing path filtering of agent-to-controller access control in Jenkins

@@ -215,11 +215,6 @@ public final class FilePath implements SerializableOnlyOverRemoting {
      */
     private static final int MAX_REDIRECTS = 20;
  
-    /**
-     * Escape hatch for some additional protections against sending callables intended to be locally used only
-     */
-    private static /* non-final for Groovy */ boolean REJECT_LOCAL_CALLABLE_DESERIALIZATION = SystemProperties.getBoolean(FilePath.class.getName() + ".rejectLocalCallableDeserialization", true);
- 
     /**
      * When this {@link FilePath} represents the remote path,
      * this field is always non-null on the controller (the field represents
@@ -601,13 +596,6 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException, Interru
             return null;
         }
         private static final long serialVersionUID = 1L;
- 
-        protected Object readResolve() {
-            if (REJECT_LOCAL_CALLABLE_DESERIALIZATION) {
-                throw new IllegalStateException("This callable is not intended to be sent through a channel");
-            }
-            return this;
-        }
     }
  
     /**
@@ -660,13 +648,6 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException, Interru
             return null;
         }
         private static final long serialVersionUID = 1L;
- 
-        protected Object readResolve() {
-            if (REJECT_LOCAL_CALLABLE_DESERIALIZATION) {
-                throw new IllegalStateException("This callable is not intended to be sent through a channel");
-            }
-            return this;
-        }
     }
  
     /**

@@ -21,6 +21,7 @@
 import hudson.model.Node;
 import hudson.model.TaskListener;
 import hudson.remoting.VirtualChannel;
+import hudson.slaves.DumbSlave;
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
@@ -809,6 +810,22 @@ public Object call() throws Exception {
  
     // --------
  
+    // Misc tests
+ 
+    @LocalData
+    @Test
+    public void testRemoteLocalUnzip() throws Exception {
+        final DumbSlave onlineSlave = j.createOnlineSlave();
+        final File zipFile = new File(j.jenkins.getRootDir(), "file.zip");
+        assertTrue(zipFile.isFile());
+        final FilePath agentRootPath = onlineSlave.getRootPath();
+        final FilePath agentZipPath = agentRootPath.child("file.zip");
+        new FilePath(zipFile).copyTo(agentZipPath);
+        agentZipPath.unzip(agentRootPath);
+    }
+ 
+    // --------
+ 
     // Utility functions
  
     protected static FilePath toFilePathOnController(File file) {

@@ -48,7 +48,9 @@ protected FilePathRule parse(String line) {
         if (line.isEmpty())     return null;
  
         // TODO This does not support custom build dir configuration (Jenkins#getRawBuildsDir() etc.)
-        line = line.replace("<BUILDDIR>","<JOBDIR>/builds/<BUILDID>");
+        line = line.replace("<BUILDDIR>","<JOBDIR>/builds/[0-9]+");
+ 
+        // Kept only for compatibility with custom user-provided rules:
         line = line.replace("<BUILDID>","(?:[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]_[0-9][0-9]-[0-9][0-9]-[0-9][0-9]|[0-9]+)");
         line = line.replace("<JOBDIR>","<JENKINS_HOME>/jobs/.+");
         final File jenkinsHome = Jenkins.get().getRootDir();

@@ -38,8 +38,12 @@ deny all <BUILDDIR>/checkpoints($|/.*)
 # But not allowing deletion to prevent data loss and symlink to prevent jailbreaking.
 allow create,mkdirs,read,stat,write <BUILDDIR>/.+
  
-# cobertura also writes out annotated sources to a dir under the job:
-allow create,mkdirs,read,stat,write <JENKINS_HOME>/jobs/.+/cobertura.*
+# cobertura also writes out annotated sources to a dir under the Maven module:
+allow create,mkdirs,read,stat,write <JOBDIR>/modules/([^/]+)/cobertura($|/.*)
+ 
+# Some maven-plugin reporters also create content outside of build directories (including one in cobertura but that is covered above):
+allow create,mkdirs,read,stat,write <JENKINS_HOME>(/jobs/([^/]+))+(|/modules/([^/]+))/(javadoc|test-javadoc)($|/.*)
+allow create,mkdirs,read,stat,write <JENKINS_HOME>(/jobs/([^/]+))+/site($|/.*)
  
 # all the other accesses that aren't specified here will be left up to other rules in this directory.
 # if no rules in those other files matches, then the access will be rejected.

@@ -17,6 +17,7 @@
 import hudson.Util;
 import hudson.model.Cause;
 import hudson.model.FreeStyleBuild;
+import hudson.model.FreeStyleProject;
 import hudson.model.Node;
 import hudson.model.TaskListener;
 import hudson.remoting.VirtualChannel;
@@ -44,6 +45,7 @@
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.LoggerRule;
+import org.jvnet.hudson.test.MockFolder;
 import org.jvnet.hudson.test.recipes.LocalData;
  
 @SuppressWarnings("ThrowableNotThrown")
@@ -719,6 +721,94 @@ public Integer call() throws Exception {
  
     // --------
  
+    @Issue("SECURITY-2455") // general issue -- Maven Projects would no longer be allowed to perform some actions
+    @Test
+    public void testMavenReportersAllowListForTopLevelJob() throws Exception {
+        final FreeStyleProject project = j.createFreeStyleProject();
+        final File topLevelProjectDir = project.getRootDir();
+ 
+        // similar but wrong names:
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(topLevelProjectDir, "not-site"))));
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(topLevelProjectDir, "not-javadoc"))));
+ 
+        // project-level archived stuff:
+        invokeOnAgent(new MkDirsWriter(new File(topLevelProjectDir, "javadoc")));
+        invokeOnAgent(new MkDirsWriter(new File(topLevelProjectDir, "test-javadoc")));
+        invokeOnAgent(new MkDirsWriter(new File(topLevelProjectDir, "site")));
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(topLevelProjectDir, "cobertura"))));
+ 
+        // cannot mkdirs this from agent:
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(topLevelProjectDir, "modules"))));
+ 
+        final File mavenModuleDir = new File(topLevelProjectDir, "modules/pretend-maven-module");
+        assertTrue(mavenModuleDir.mkdirs());
+ 
+        // module-level archived stuff:
+        invokeOnAgent(new MkDirsWriter(new File(mavenModuleDir, "javadoc")));
+        invokeOnAgent(new MkDirsWriter(new File(mavenModuleDir, "test-javadoc")));
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(mavenModuleDir, "site"))));
+        invokeOnAgent(new MkDirsWriter(new File(mavenModuleDir, "cobertura")));
+    }
+ 
+    @Issue("SECURITY-2455") // general issue -- Maven Projects would no longer be allowed to perform some actions
+    @Test
+    public void testMavenReportersAllowListForJobInFolder() throws Exception {
+        final MockFolder theFolder = j.createFolder("theFolder");
+        {
+            // basic child job
+            final FreeStyleProject childProject = theFolder.createProject(FreeStyleProject.class, "child");
+            final File childProjectRootDir = childProject.getRootDir();
+ 
+            // project-level archived stuff for child project inside folder:
+            invokeOnAgent(new MkDirsWriter(new File(childProjectRootDir, "javadoc")));
+            invokeOnAgent(new MkDirsWriter(new File(childProjectRootDir, "test-javadoc")));
+            invokeOnAgent(new MkDirsWriter(new File(childProjectRootDir, "site")));
+            assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(childProjectRootDir, "cobertura"))));
+        }
+ 
+        { // misleadingly named child job (like one of the approved folders):
+            final FreeStyleProject siteChildProject = theFolder.createProject(FreeStyleProject.class, "site");
+            final File siteChildProjectRootDir = siteChildProject.getRootDir();
+ 
+            // cannot mkdirs this from agent despite 'site' in the path (but on wrong level):
+            assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(siteChildProjectRootDir)));
+            assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(siteChildProjectRootDir, "foo"))));
+            assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(siteChildProjectRootDir, "modules"))));
+ 
+            // project-level archived stuff for another child inside folder:
+            invokeOnAgent(new MkDirsWriter(new File(siteChildProjectRootDir, "javadoc")));
+            invokeOnAgent(new MkDirsWriter(new File(siteChildProjectRootDir, "test-javadoc")));
+            invokeOnAgent(new MkDirsWriter(new File(siteChildProjectRootDir, "site")));
+            assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(siteChildProjectRootDir, "cobertura"))));
+ 
+            final File childProjectMavenModuleDir = new File(siteChildProjectRootDir, "modules/pretend-maven-module");
+            assertTrue(childProjectMavenModuleDir.mkdirs());
+ 
+            // module-level archived stuff:
+            invokeOnAgent(new MkDirsWriter(new File(childProjectMavenModuleDir, "javadoc")));
+            invokeOnAgent(new MkDirsWriter(new File(childProjectMavenModuleDir, "test-javadoc")));
+            assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkDirsWriter(new File(childProjectMavenModuleDir, "site"))));
+            invokeOnAgent(new MkDirsWriter(new File(childProjectMavenModuleDir, "cobertura")));
+        }
+    }
+ 
+    private static class MkDirsWriter extends MasterToSlaveCallable<Object, Exception> {
+        private final File root;
+ 
+        private MkDirsWriter(File root) {
+            this.root = root;
+        }
+ 
+        @Override
+        public Object call() throws Exception {
+            toFilePathOnController(root).mkdirs();
+            toFilePathOnController(new File(root, "file.txt")).write("text", "UTF-8");
+            return null;
+        }
+    }
+ 
+    // --------
+ 
     // Utility functions
  
     protected static FilePath toFilePathOnController(File file) {
@@ -730,8 +820,12 @@ protected static FilePath toFilePathOnController(String path) {
         return new FilePath(channel, path);
     }
  
+    protected Node agent;
+ 
     protected  <T, X extends Throwable> T invokeOnAgent(MasterToSlaveCallable<T, X> callable) throws Exception, X {
-        final Node agent = j.createOnlineSlave();
+        if (agent == null) {
+            agent = j.createOnlineSlave();
+        }
         return Objects.requireNonNull(agent.getChannel()).call(callable);
     }
  

@@ -34,7 +34,6 @@
 import com.jcraft.jzlib.GZIPOutputStream;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
-import edu.umd.cs.findbugs.annotations.Nullable;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.Launcher.LocalLauncher;
 import hudson.Launcher.RemoteLauncher;
@@ -216,6 +215,11 @@ public final class FilePath implements SerializableOnlyOverRemoting {
      */
     private static final int MAX_REDIRECTS = 20;
  
+    /**
+     * Escape hatch for some additional protections against sending callables intended to be locally used only
+     */
+    private static /* non-final for Groovy */ boolean REJECT_LOCAL_CALLABLE_DESERIALIZATION = SystemProperties.getBoolean(FilePath.class.getName() + ".rejectLocalCallableDeserialization", true);
+ 
     /**
      * When this {@link FilePath} represents the remote path,
      * this field is always non-null on the controller (the field represents
@@ -239,18 +243,6 @@ public final class FilePath implements SerializableOnlyOverRemoting {
      */
     private /*final*/ String remote;
  
-    /**
-     * If this {@link FilePath} is deserialized to handle file access request from a remote computer,
-     * this field is set to the filter that performs access control.
-     *
-     * <p>
-     * If null, no access control is needed.
-     *
-     * @see #filterNonNull()
-     */
-    private transient @Nullable
-    SoloFilePathFilter filter;
- 
     /**
      * Creates a {@link FilePath} that represents a path on the given node.
      *
@@ -527,7 +519,7 @@ public int archive(final ArchiverFactory factory, OutputStream os, final DirScan
         final OutputStream out = channel != null ? new RemoteOutputStream(os) : os;
         return act(new Archive(factory, out, scanner, verificationRoot, noFollowLinks));
     }
-    private class Archive extends SecureFileCallable<Integer> {
+    private static class Archive extends SecureFileCallable<Integer> {
         private final ArchiverFactory factory;
         private final OutputStream out;
         private final DirScanner scanner;
@@ -573,14 +565,14 @@ public int archive(final ArchiverFactory factory, OutputStream os, final String
      */
     public void unzip(final FilePath target) throws IOException, InterruptedException {
         // TODO: post release, re-unite two branches by introducing FileStreamCallable that resolves InputStream
-        if (this.channel!=target.channel) {// local -> remote or remote->local
+        if (channel != target.channel) {// local -> remote or remote->local
             final RemoteInputStream in = new RemoteInputStream(read(), Flag.GREEDY);
             target.act(new UnzipRemote(in));
         } else {// local -> local or remote->remote
-            target.act(new UnzipLocal());
+            target.act(new UnzipLocal(this));
         }
     }
-    private class UnzipRemote extends SecureFileCallable<Void> {
+    private static class UnzipRemote extends SecureFileCallable<Void> {
         private final RemoteInputStream in;
         UnzipRemote(RemoteInputStream in) {
             this.in = in;
@@ -592,14 +584,30 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException, Interru
         }
         private static final long serialVersionUID = 1L;
     }
-    private class UnzipLocal extends SecureFileCallable<Void> {
+    private static class UnzipLocal extends SecureFileCallable<Void> {
+ 
+        private final FilePath filePath;
+ 
+        private UnzipLocal(FilePath filePath) {
+            this.filePath = filePath;
+        }
+ 
         @Override
         public Void invoke(File dir, VirtualChannel channel) throws IOException, InterruptedException {
-            assert !FilePath.this.isRemote();       // this.channel==target.channel above
-            unzip(dir, reading(new File(FilePath.this.getRemote()))); // shortcut to local file
+            if (this.filePath.isRemote()) {
+                throw new IllegalStateException("Expected local path for file: " + filePath); // this.channel==target.channel above
+            }
+            unzip(dir, reading(new File(this.filePath.getRemote()))); // shortcut to local file
             return null;
         }
         private static final long serialVersionUID = 1L;
+ 
+        protected Object readResolve() {
+            if (REJECT_LOCAL_CALLABLE_DESERIALIZATION) {
+                throw new IllegalStateException("This callable is not intended to be sent through a channel");
+            }
+            return this;
+        }
     }
  
     /**
@@ -613,39 +621,52 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException, Interru
      * @see #untarFrom(InputStream, TarCompression)
      */
     public void untar(final FilePath target, final TarCompression compression) throws IOException, InterruptedException {
+        final FilePath source = FilePath.this;
         // TODO: post release, re-unite two branches by introducing FileStreamCallable that resolves InputStream
-        if (this.channel!=target.channel) {// local -> remote or remote->local
-            final RemoteInputStream in = new RemoteInputStream(read(), Flag.GREEDY);
-            target.act(new UntarRemote(compression, in));
+        if (source.channel != target.channel) {// local -> remote or remote->local
+            final RemoteInputStream in = new RemoteInputStream(source.read(), Flag.GREEDY);
+            target.act(new UntarRemote(source.getName(), compression, in));
         } else {// local -> local or remote->remote
-            target.act(new UntarLocal(compression));
+            target.act(new UntarLocal(source, compression));
         }
     }
-    private class UntarRemote extends SecureFileCallable<Void> {
+    private static class UntarRemote extends SecureFileCallable<Void> {
         private final TarCompression compression;
         private final RemoteInputStream in;
-        UntarRemote(TarCompression compression, RemoteInputStream in) {
+        private final String name;
+        UntarRemote(String name, TarCompression compression, RemoteInputStream in) {
             this.compression = compression;
             this.in = in;
+            this.name = name;
         }
         @Override
         public Void invoke(File dir, VirtualChannel channel) throws IOException, InterruptedException {
-            readFromTar(FilePath.this.getName(), dir, compression.extract(in));
+            readFromTar(name, dir, compression.extract(in));
             return null;
         }
         private static final long serialVersionUID = 1L;
     }
-    private class UntarLocal extends SecureFileCallable<Void> {
+    private static class UntarLocal extends SecureFileCallable<Void> {
         private final TarCompression compression;
-        UntarLocal(TarCompression compression) {
+        private final FilePath filePath;
+ 
+        UntarLocal(FilePath source, TarCompression compression) {
+            this.filePath = source;
             this.compression = compression;
         }
         @Override
         public Void invoke(File dir, VirtualChannel channel) throws IOException, InterruptedException {
-            readFromTar(FilePath.this.getName(), dir, compression.extract(FilePath.this.read()));
+            readFromTar(this.filePath.getName(), dir, compression.extract(this.filePath.read()));
             return null;
         }
         private static final long serialVersionUID = 1L;
+ 
+        protected Object readResolve() {
+            if (REJECT_LOCAL_CALLABLE_DESERIALIZATION) {
+                throw new IllegalStateException("This callable is not intended to be sent through a channel");
+            }
+            return this;
+        }
     }
  
     /**
@@ -660,7 +681,7 @@ public void unzipFrom(InputStream _in) throws IOException, InterruptedException
         final InputStream in = new RemoteInputStream(_in, Flag.GREEDY);
         act(new UnzipFrom(in));
     }
-    private class UnzipFrom extends SecureFileCallable<Void> {
+    private static class UnzipFrom extends SecureFileCallable<Void> {
         private final InputStream in;
         UnzipFrom(InputStream in) {
             this.in = in;
@@ -673,7 +694,7 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException {
         private static final long serialVersionUID = 1L;
     }
  
-    private void unzip(File dir, InputStream in) throws IOException {
+    private static void unzip(File dir, InputStream in) throws IOException {
         File tmpFile = File.createTempFile("tmpzip", null); // uses java.io.tmpdir
         try {
             // TODO why does this not simply use ZipInputStream?
@@ -685,7 +706,7 @@ private void unzip(File dir, InputStream in) throws IOException {
         }
     }
  
-    private void unzip(File dir, File zipFile) throws IOException {
+    private static void unzip(File dir, File zipFile) throws IOException {
         dir = dir.getAbsoluteFile();    // without absolutization, getParentFile below seems to fail
         ZipFile zip = new ZipFile(zipFile);
         Enumeration<ZipEntry> entries = zip.getEntries();
@@ -734,7 +755,7 @@ private static class Absolutize extends SecureFileCallable<String> {
         private static final long serialVersionUID = 1L;
         @Override
         public String invoke(File f, VirtualChannel channel) throws IOException {
-            return f.getAbsolutePath();
+            return stating(f).getAbsolutePath();
         }
     }
  
@@ -754,7 +775,7 @@ private static class HasSymlink extends SecureFileCallable<Boolean> {
  
         @Override
         public Boolean invoke(File f, VirtualChannel channel) throws IOException {
-            return isSymlink(f, verificationRoot, noFollowLinks);
+            return isSymlink(stating(f), verificationRoot, noFollowLinks);
         }
     }
  
@@ -792,7 +813,7 @@ public boolean accept(File file) {
     public void symlinkTo(final String target, final TaskListener listener) throws IOException, InterruptedException {
         act(new SymlinkTo(target, listener));
     }
-    private class SymlinkTo extends SecureFileCallable<Void> {
+    private static class SymlinkTo extends SecureFileCallable<Void> {
         private final String target;
         private final TaskListener listener;
         SymlinkTo(String target, TaskListener listener) {
@@ -802,8 +823,7 @@ private class SymlinkTo extends SecureFileCallable<Void> {
         private static final long serialVersionUID = 1L;
         @Override
         public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
-            symlinking(f);
-            Util.createSymlink(f.getParentFile(), target, f.getName(), listener);
+            Util.createSymlink(symlinking(f).getParentFile(), target, f.getName(), listener);
             return null;
         }
     }
@@ -818,7 +838,7 @@ public Void invoke(File f, VirtualChannel channel) throws IOException, Interrupt
     public String readLink() throws IOException, InterruptedException {
         return act(new ReadLink());
     }
-    private class ReadLink extends SecureFileCallable<String> {
+    private static class ReadLink extends SecureFileCallable<String> {
         private static final long serialVersionUID = 1L;
         @Override
         public String invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
@@ -896,7 +916,7 @@ public void untarFrom(InputStream _in, final TarCompression compression) throws
             _in.close();
         }
     }
-    private class UntarFrom extends SecureFileCallable<Void> {
+    private static class UntarFrom extends SecureFileCallable<Void> {
         private final TarCompression compression;
         private final InputStream in;
         UntarFrom(TarCompression compression, InputStream in) {
@@ -905,7 +925,7 @@ private class UntarFrom extends SecureFileCallable<Void> {
         }
         @Override
         public Void invoke(File dir, VirtualChannel channel) throws IOException {
-            readFromTar("input stream",dir, compression.extract(in));
+            readFromTar("input stream",dir, compression.extract(in)); // #writing etc. are called in #readFromTar
             return null;
         }
         private static final long serialVersionUID = 1L;
@@ -1157,7 +1177,7 @@ private <T> T act(final FileCallable<T> callable, ClassLoader cl) throws IOExcep
         if(channel!=null) {
             // run this on a remote system
             try {
-                DelegatingCallable<T,IOException> wrapper = new FileCallableWrapper<>(callable, cl);
+                DelegatingCallable<T,IOException> wrapper = new FileCallableWrapper<>(callable, cl, this);
                 for (FileCallableWrapperFactory factory : ExtensionList.lookup(FileCallableWrapperFactory.class)) {
                     wrapper = factory.wrap(wrapper);
                 }
@@ -1233,7 +1253,7 @@ protected void after() {}
      */
     public <T> Future<T> actAsync(final FileCallable<T> callable) throws IOException, InterruptedException {
         try {
-            DelegatingCallable<T,IOException> wrapper = new FileCallableWrapper<>(callable);
+            DelegatingCallable<T,IOException> wrapper = new FileCallableWrapper<>(callable, this);
             for (FileCallableWrapperFactory factory : ExtensionList.lookup(FileCallableWrapperFactory.class)) {
                 wrapper = factory.wrap(wrapper);
             }
@@ -1302,7 +1322,7 @@ private static class ToURI extends SecureFileCallable<URI> {
         private static final long serialVersionUID = 1L;
         @Override
         public URI invoke(File f, VirtualChannel channel) {
-            return f.toURI();
+            return stating(f).toURI();
         }
     }
  
@@ -1340,7 +1360,7 @@ public void mkdirs() throws IOException, InterruptedException {
             throw new IOException("Failed to mkdirs: " + remote);
         }
     }
-    private class Mkdirs extends SecureFileCallable<Boolean> {
+    private static class Mkdirs extends SecureFileCallable<Boolean> {
         private static final long serialVersionUID = 1L;
         @Override
         public Boolean invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
@@ -1366,7 +1386,7 @@ public void deleteSuffixesRecursive() throws IOException, InterruptedException {
     /**
      * Deletes all suffixed directories that are separated by {@link WorkspaceList#COMBINATOR}, including all its contents recursively.
      */
-    private class DeleteSuffixesRecursive extends SecureFileCallable<Void> {
+    private static class DeleteSuffixesRecursive extends SecureFileCallable<Void> {
         private static final long serialVersionUID = 1L;
  
         @Override
@@ -1398,7 +1418,7 @@ private static File[] listParentFiles(File f) {
     public void deleteRecursive() throws IOException, InterruptedException {
         act(new DeleteRecursive());
     }
-    private class DeleteRecursive extends SecureFileCallable<Void> {
+    private static class DeleteRecursive extends SecureFileCallable<Void> {
         private static final long serialVersionUID = 1L;
         @Override
         public Void invoke(File f, VirtualChannel channel) throws IOException {
@@ -1413,7 +1433,7 @@ public Void invoke(File f, VirtualChannel channel) throws IOException {
     public void deleteContents() throws IOException, InterruptedException {
         act(new DeleteContents());
     }
-    private class DeleteContents extends SecureFileCallable<Void> {
+    private static class DeleteContents extends SecureFileCallable<Void> {
         private static final long serialVersionUID = 1L;
         @Override
         public Void invoke(File f, VirtualChannel channel) throws IOException {
@@ -1516,7 +1536,7 @@ public FilePath createTempFile(final String prefix, final String suffix) throws
             throw new IOException("Failed to create a temp file on "+remote,e);
         }
     }
-    private class CreateTempFile extends SecureFileCallable<String> {
+    private static class CreateTempFile extends SecureFileCallable<String> {
         private final String prefix;
         private final String suffix;
         CreateTempFile(String prefix, String suffix) {
@@ -1526,7 +1546,8 @@ private class CreateTempFile extends SecureFileCallable<String> {
         private static final long serialVersionUID = 1L;
         @Override
         public String invoke(File dir, VirtualChannel channel) throws IOException {
-            File f = writing(File.createTempFile(prefix, suffix, dir));
+            creating(new File(dir, prefix + "-security-check-dummy-" + suffix)); // use fake file to check access before creation
+            File f = creating(File.createTempFile(prefix, suffix, dir));
             return f.getName();
         }
     }
@@ -1580,7 +1601,7 @@ public FilePath createTextTempFile(final String prefix, final String suffix, fin
             throw new IOException("Failed to create a temp file on "+remote,e);
         }
     }
-    private final class CreateTextTempFile extends SecureFileCallable<String> {
+    private static class CreateTextTempFile extends SecureFileCallable<String> {
         private static final long serialVersionUID = 1L;
         private final boolean inThisDirectory;
         private final String prefix;
@@ -1601,6 +1622,7 @@ public String invoke(File dir, VirtualChannel channel) throws IOException {
  
             File f;
             try {
+                creating(new File(dir, prefix + "-security-check-dummy-" + suffix)); // use fake file to check access before creation
                 f = creating(File.createTempFile(prefix, suffix, dir));
             } catch (IOException e) {
                 throw new IOException("Failed to create a temporary directory in "+dir,e);
@@ -1642,7 +1664,7 @@ public FilePath createTempDir(final String prefix, final String suffix) throws I
             throw new IOException("Failed to create a temp directory on "+remote,e);
         }
     }
-    private class CreateTempDir extends SecureFileCallable<String> {
+    private static class CreateTempDir extends SecureFileCallable<String> {
         private final String name;
         CreateTempDir(String name) {
             this.name = name;
@@ -1650,6 +1672,7 @@ private class CreateTempDir extends SecureFileCallable<String> {
             private static final long serialVersionUID = 1L;
             @Override
             public String invoke(File dir, VirtualChannel channel) throws IOException {
+                mkdirsing(new File(dir, name + "-security-test")); // ensure access
  
                 Path tempPath;
                 final boolean isPosix = FileSystems.getDefault().supportedFileAttributeViews().contains("posix");
@@ -1661,8 +1684,8 @@ public String invoke(File dir, VirtualChannel channel) throws IOException {
                     tempPath = Files.createTempDirectory(Util.fileToPath(dir), name);
                 }
  
-                if (tempPath.toFile() == null) {
-                    throw new IOException("Failed to obtain file from path " + dir + " on " + remote);
+                if (mkdirsing(tempPath.toFile()) == null) {
+                    throw new IOException("Failed to obtain file from path " + dir);
                 }
                 return tempPath.toFile().getName();
             }
@@ -1677,7 +1700,7 @@ public boolean delete() throws IOException, InterruptedException {
         act(new Delete());
         return true;
     }
-    private class Delete extends SecureFileCallable<Void> {
+    private static class Delete extends SecureFileCallable<Void> {
         private static final long serialVersionUID = 1L;
         @Override
         public Void invoke(File f, VirtualChannel channel) throws IOException {
@@ -1692,7 +1715,7 @@ public Void invoke(File f, VirtualChannel channel) throws IOException {
     public boolean exists() throws IOException, InterruptedException {
         return act(new Exists());
     }
-    private class Exists extends SecureFileCallable<Boolean> {
+    private static class Exists extends SecureFileCallable<Boolean> {
         private static final long serialVersionUID = 1L;
         @Override
         public Boolean invoke(File f, VirtualChannel channel) throws IOException {
@@ -1710,7 +1733,7 @@ public Boolean invoke(File f, VirtualChannel channel) throws IOException {
… diff truncated

@@ -1,8 +1,16 @@
 package jenkins;
  
 import edu.umd.cs.findbugs.annotations.Nullable;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.FilePath;
+import jenkins.util.SystemProperties;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+ 
 import java.io.File;
+import java.util.UUID;
+import java.util.logging.Level;
+import java.util.logging.Logger;
  
 /**
  * Variant of {@link FilePathFilter} that assumes it is the sole actor.
@@ -13,6 +21,13 @@
  * @author Kohsuke Kawaguchi
  */
 public final class SoloFilePathFilter extends FilePathFilter {
+ 
+    private static final Logger LOGGER = Logger.getLogger(SoloFilePathFilter.class.getName());
+ 
+    @SuppressFBWarnings("MS_SHOULD_BE_FINAL")
+    @Restricted(NoExternalUse.class)
+    public static /* non-final for Groovy */ boolean REDACT_ERRORS = SystemProperties.getBoolean(SoloFilePathFilter.class.getName() + ".redactErrors", true);
+ 
     private final FilePathFilter base;
  
     private SoloFilePathFilter(FilePathFilter base) {
@@ -28,8 +43,17 @@ private SoloFilePathFilter(FilePathFilter base) {
     }
  
     private boolean noFalse(String op, File f, boolean b) {
-        if (!b)
-            throw new SecurityException("agent may not " + op + " " + f+"\nSee https://www.jenkins.io/redirect/security-144 for more details");
+        if (!b) {
+            final String detailedMessage = "Agent may not '" + op + "' at '" + f + "'. See https://www.jenkins.io/redirect/security-144 for more information.";
+            if (REDACT_ERRORS) {
+                // We may end up trying to access file paths indirectly, e.g. FilePath#listFiles starts in an allowed dir but follows symlinks outside, so do not disclose paths in error message
+                UUID uuid = UUID.randomUUID();
+                LOGGER.log(Level.WARNING, () -> uuid + ": " + detailedMessage);
+                throw new SecurityException("Agent may not access a file path. See the system log for more details about the error ID '" + uuid + "' and https://www.jenkins.io/redirect/security-144 for more information.");
+            } else {
+                throw new SecurityException(detailedMessage);
+            }
+        }
         return true;
     }
     
@@ -49,12 +73,18 @@ public boolean write(File f) throws SecurityException {
  
     @Override
     public boolean symlink(File f) throws SecurityException {
-        return noFalse("symlink",f,base.write(normalize(f)));
+        return noFalse("symlink",f,base.symlink(normalize(f)));
     }
  
     @Override
     public boolean mkdirs(File f) throws SecurityException {
-        return noFalse("mkdirs",f,base.mkdirs(normalize(f)));
+        // mkdirs is special because it could operate on parents of the specified path
+        File reference = normalize(f);
+        while (reference != null && !reference.exists()) {
+            noFalse("mkdirs", f, base.mkdirs(reference)); // Pass f as reference into the error to be vague
+            reference = reference.getParentFile();
+        }
+        return true;
     }
  
     @Override

@@ -5,12 +5,16 @@
 import hudson.Functions;
 import hudson.model.Failure;
 import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import jenkins.model.Jenkins;
@@ -21,6 +25,9 @@
  * @author Kohsuke Kawaguchi
  */
 class FilePathRuleConfig extends ConfigDirectory<FilePathRule,List<FilePathRule>> {
+ 
+    private static final Logger LOGGER = Logger.getLogger(FilePathRuleConfig.class.getName());
+ 
     FilePathRuleConfig(File file) {
         super(file);
     }
@@ -40,10 +47,17 @@ protected FilePathRule parse(String line) {
         line = line.trim();
         if (line.isEmpty())     return null;
  
+        // TODO This does not support custom build dir configuration (Jenkins#getRawBuildsDir() etc.)
         line = line.replace("<BUILDDIR>","<JOBDIR>/builds/<BUILDID>");
         line = line.replace("<BUILDID>","(?:[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]_[0-9][0-9]-[0-9][0-9]-[0-9][0-9]|[0-9]+)");
         line = line.replace("<JOBDIR>","<JENKINS_HOME>/jobs/.+");
-        line = line.replace("<JENKINS_HOME>","\\Q"+Jenkins.get().getRootDir().getPath()+"\\E");
+        final File jenkinsHome = Jenkins.get().getRootDir();
+        try {
+            line = line.replace("<JENKINS_HOME>","\\Q" + jenkinsHome.getCanonicalPath() + "\\E");
+        } catch (IOException e) {
+            LOGGER.log(Level.WARNING, e, () -> "Failed to determine canonical path to Jenkins home directory, falling back to configured value: " + jenkinsHome.getPath());
+            line = line.replace("<JENKINS_HOME>","\\Q" + jenkinsHome.getPath() + "\\E");
+        }
  
         // config file is always /-separated even on Windows, so bring it back to \-separation.
         // This is done in the context of regex, so it has to be \\, which means in the source code it is \\\\
@@ -77,9 +91,11 @@ public boolean checkFileAccess(String op, File path) throws SecurityException {
         for (FilePathRule rule : get()) {
             if (rule.op.matches(op)) {
                 if (pathStr==null) {
-                    // do not canonicalize, so that JENKINS_HOME that spans across
-                    // multiple volumes via symlinks can look logically like one unit.
-                    pathStr = path.getPath();
+                    try {
+                        pathStr = path.getCanonicalPath();
+                    } catch (IOException ex) {
+                        throw new UncheckedIOException(ex);
+                    }
                     if (isWindows())    // Windows accepts '/' as separator, but for rule matching we want to normalize for consistent comparison
                         pathStr = pathStr.replace('/','\\');
                 }

@@ -0,0 +1,756 @@
+package jenkins.security;
+ 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeFalse;
+import static org.jvnet.hudson.test.LoggerRule.recorded;
+ 
+import hudson.ExtensionList;
+import hudson.FilePath;
+import hudson.Functions;
+import hudson.Util;
+import hudson.model.Cause;
+import hudson.model.FreeStyleBuild;
+import hudson.model.Node;
+import hudson.model.TaskListener;
+import hudson.remoting.VirtualChannel;
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.lang.reflect.Constructor;
+import java.net.URI;
+import java.nio.file.Files;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.logging.Level;
+import jenkins.SlaveToMasterFileCallable;
+import jenkins.SoloFilePathFilter;
+import jenkins.agents.AgentComputerUtil;
+import jenkins.security.s2m.AdminWhitelistRule;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.io.output.NullOutputStream;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.function.ThrowingRunnable;
+import org.jvnet.hudson.test.FlagRule;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
+import org.jvnet.hudson.test.recipes.LocalData;
+ 
+@SuppressWarnings("ThrowableNotThrown")
+@Issue("SECURITY-2455")
+public class Security2455Test {
+ 
+    // TODO After merge, reference the class directly
+    private static final String SECURITY_2428_KILLSWITCH = "jenkins.security.s2m.RunningBuildFilePathFilter.FAIL";
+ 
+    @Rule
+    public final FlagRule<String> flagRule = FlagRule.systemProperty(SECURITY_2428_KILLSWITCH, "false");
+ 
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+ 
+    @Rule
+    public LoggerRule logging = new LoggerRule().record(SoloFilePathFilter.class, Level.WARNING);
+ 
+    @Before
+    public void setup() {
+        ExtensionList.lookupSingleton(AdminWhitelistRule.class).setMasterKillSwitch(false);
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2427")
+    public void mkdirsParentsTest() {
+        final File buildStuff = new File(j.jenkins.getRootDir(), "job/nonexistent/builds/1/foo/bar");
+        logging.capture(10);
+        SecurityException ex = assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkdirsParentsCallable(buildStuff)));
+        assertThat(logging, recorded(containsString("foo/bar")));
+        assertThat(ex.getMessage(), not(containsString("foo/bar"))); // test error redaction
+ 
+        SoloFilePathFilter.REDACT_ERRORS = false;
+        try {
+            SecurityException ex2 = assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new MkdirsParentsCallable(buildStuff)));
+            assertThat(ex2.getMessage(), containsString("foo/bar")); // test error redaction
+        } finally {
+            SoloFilePathFilter.REDACT_ERRORS = true;
+        }
+    }
+    private static class MkdirsParentsCallable extends MasterToSlaveCallable<String, Exception> {
+        private final File file;
+ 
+        private MkdirsParentsCallable(File file) {
+            this.file = file;
+        }
+ 
+        @Override
+        public String call() throws Exception {
+            toFilePathOnController(this.file).mkdirs();
+            return null;
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2444")
+    public void testNonCanonicalPath() throws Exception {
+        assumeFalse(Functions.isWindows());
+        final FreeStyleBuild build = j.createFreeStyleProject().scheduleBuild2(0, new Cause.UserIdCause()).waitForStart();
+        j.waitForCompletion(build);
+        final File link = new File(build.getRootDir(), "link");
+        final File secrets = new File(j.jenkins.getRootDir(), "secrets/master.key");
+        Files.createSymbolicLink(link.toPath(), secrets.toPath());
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new ReadToStringCallable(link)));
+    }
+    @Test
+    @Issue("SECURITY-2444")
+    public void testNonCanonicalPathOnController() throws Exception {
+        assumeFalse(Functions.isWindows());
+        final FreeStyleBuild build = j.createFreeStyleProject().scheduleBuild2(0, new Cause.UserIdCause()).waitForStart();
+        j.waitForCompletion(build);
+        final File link = new File(build.getRootDir(), "link");
+        final File secrets = new File(j.jenkins.getRootDir(), "secrets/master.key");
+        Files.createSymbolicLink(link.toPath(), secrets.toPath());
+        String result = FilePath.localChannel.call(new ReadToStringCallable(link));
+        assertEquals(IOUtils.readLines(new FileReader(secrets)).get(0), result);
+    }
+ 
+    private static class ReadToStringCallable extends MasterToSlaveCallable<String, Exception> {
+ 
+        final String abs;
+ 
+        ReadToStringCallable(File link) {
+            abs = link.getPath();
+        }
+ 
+        @Override
+        public String call() throws IOException {
+            FilePath p = toFilePathOnController(new File(abs));
+            try {
+                return p.readToString();
+            } catch (InterruptedException e) {
+                throw new IOException(e);
+            }
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue({"SECURITY-2446", "SECURITY-2531"})
+    // $ tar tvf symlink.tar
+    // lrwxr-xr-x  0 501    20          0 Oct  5 09:50 foo -> ../../../../secrets
+    @LocalData
+    public void testUntaringSymlinksFails() throws Exception {
+        final FreeStyleBuild freeStyleBuild = j.buildAndAssertSuccess(j.createFreeStyleProject());
+        final File symlinkTarFile = new File(j.jenkins.getRootDir(), "symlink.tar");
+        final File untarTargetFile = new File(freeStyleBuild.getRootDir(), "foo");
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new UntarFileCallable(symlinkTarFile, untarTargetFile)));
+    }
+    private static final class UntarFileCallable extends MasterToSlaveCallable<Integer, Exception> {
+        private final File source;
+        private final File destination;
+ 
+        private UntarFileCallable(File source, File destination) {
+            this.source = source;
+            this.destination = destination;
+        }
+ 
+        @Override
+        public Integer call() throws Exception {
+            final FilePath sourceFilePath = new FilePath(source);
+            final FilePath destinationFilePath = toFilePathOnController(destination);
+            sourceFilePath.untar(destinationFilePath, FilePath.TarCompression.NONE);
+            return 1;
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2453")
+    public void testTarSymlinksThatAreSafe() throws Exception {
+        assumeFalse(Functions.isWindows());
+        final File buildDir = j.buildAndAssertSuccess(j.createFreeStyleProject()).getRootDir();
+        // We cannot touch the build dir itself
+        final File innerDir = new File(buildDir, "dir");
+        final File innerDir2 = new File(buildDir, "dir2");
+        assertTrue(innerDir.mkdirs());
+        assertTrue(innerDir2.mkdirs());
+        assertTrue(new File(innerDir2, "the-file").createNewFile());
+        Util.createSymlink(innerDir, "../dir2", "link", TaskListener.NULL);
+        assertTrue(new File(innerDir, "link/the-file").exists());
+        final int files = invokeOnAgent(new TarCaller(innerDir));
+        assertEquals(1, files);
+    }
+    @Test
+    @Issue("SECURITY-2453")
+    public void testTarSymlinksOutsideAllowedDirs() throws Exception {
+        assumeFalse(Functions.isWindows());
+        final File buildDir = j.buildAndAssertSuccess(j.createFreeStyleProject()).getRootDir();
+        // We cannot touch the build dir itself
+        final File innerDir = new File(buildDir, "dir");
+        assertTrue(innerDir.mkdirs());
+        Util.createSymlink(innerDir, "../../../../../secrets", "secrets-link", TaskListener.NULL);
+        assertTrue(new File(innerDir, "secrets-link/master.key").exists());
+        logging.capture(10);
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new TarCaller(innerDir)));
+        assertThat(logging, recorded(containsString("filepath-filters.d")));
+    }
+ 
+    private static class TarCaller extends MasterToSlaveCallable<Integer, Exception> {
+        private final File root;
+ 
+        private TarCaller(File root) {
+            this.root = root;
+        }
+ 
+        @Override
+        public Integer call() throws Exception {
+            return toFilePathOnController(root).tar(NullOutputStream.NULL_OUTPUT_STREAM, "**");
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2484")
+    public void zipTest() {
+        final File secrets = new File(j.jenkins.getRootDir(), "secrets");
+        assertTrue(secrets.exists());
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new ZipTestCallable(secrets)));
+    }
+    @Test
+    @Issue("SECURITY-2484")
+    public void zipTestController() throws Exception {
+        final File secrets = new File(j.jenkins.getRootDir(), "secrets");
+        assertTrue(secrets.exists());
+        FilePath.localChannel.call(new ZipTestCallable(secrets));
+    }
+ 
+    private static class ZipTestCallable extends MasterToSlaveCallable<String, Exception> {
+        private final File file;
+ 
+        private ZipTestCallable(File file) {
+            this.file = file;
+        }
+ 
+        @Override
+        public String call() throws Exception {
+            final File tmp = File.createTempFile("security2455_", ".zip");
+            tmp.deleteOnExit();
+            toFilePathOnController(file).zip(new FilePath(tmp));
+            return tmp.getName();
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2485")
+    @LocalData
+    public void unzipRemoteTest() {
+        final File targetDir = j.jenkins.getRootDir();
+        final File source = new File(targetDir, "file.zip"); // in this test, controller and agent are on same FS so this works -- file needs to exist but content should not be read
+        assertTrue(targetDir.exists());
+        final List<File> filesBefore = Arrays.asList(Objects.requireNonNull(targetDir.listFiles()));
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new UnzipRemoteTestCallable(targetDir, source)));
+        final List<File> filesAfter = Arrays.asList(Objects.requireNonNull(targetDir.listFiles()));
+        // We cannot do a direct comparison here because `logs/` appears during execution
+        assertEquals(filesBefore.size(), filesAfter.stream().filter(it -> !it.getName().equals("logs")).count());
+    }
+ 
+    private static class UnzipRemoteTestCallable extends MasterToSlaveCallable<String, Exception> {
+        private final File destination;
+        private final File source;
+ 
+        private UnzipRemoteTestCallable(File destination, File source) {
+            this.destination = destination;
+            this.source = source;
+        }
+ 
+        @Override
+        public String call() throws Exception {
+            FilePath onAgent = new FilePath(source);
+            onAgent.unzip(toFilePathOnController(destination));
+            return null;
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2485")
+    public void testCopyRecursiveFromControllerToAgent() {
+        IOException ex = assertThrowsIOExceptionCausedBy(IOException.class, () -> invokeOnAgent(new CopyRecursiveToFromControllerToAgentCallable(new FilePath(new File(j.jenkins.getRootDir(), "secrets")))));
+        assertThat(Objects.requireNonNull(ex).getMessage(), containsString("Unexpected end of ZLIB input stream")); // TODO this used to say "premature", why the change?
+    }
+    private static class CopyRecursiveToFromControllerToAgentCallable extends MasterToSlaveCallable<Integer, Exception> {
+        private final FilePath controllerFilePath;
+ 
+        private CopyRecursiveToFromControllerToAgentCallable(FilePath controllerFilePath) {
+            this.controllerFilePath = controllerFilePath;
+        }
+ 
+        @Override
+        public Integer call() throws Exception {
+            return controllerFilePath.copyRecursiveTo(new FilePath(Files.createTempDirectory("jenkins-test").toFile()));
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2485")
+    public void testCopyRecursiveFromAgentToController() {
+        assertThrowsIOExceptionCausedBy(SecurityException.class, () -> invokeOnAgent(new CopyRecursiveToFromAgentToControllerCallable(new FilePath(new File(j.jenkins.getRootDir(), "secrets")))));
+    }
+    private static class CopyRecursiveToFromAgentToControllerCallable extends MasterToSlaveCallable<Integer, Exception> {
+        private final FilePath controllerFilePath;
+ 
+        private CopyRecursiveToFromAgentToControllerCallable(FilePath controllerFilePath) {
+            this.controllerFilePath = controllerFilePath;
+        }
+ 
+        @Override
+        public Integer call() throws Exception {
+            final File localPath = Files.createTempDirectory("jenkins-test").toFile();
+            assertTrue(new File(localPath, "tmpfile").createNewFile());
+            return new FilePath(localPath).copyRecursiveTo(controllerFilePath);
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2486")
+    public void testDecoyWrapper() {
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new ReadToStringBypassCallable(j.jenkins.getRootDir())));
+    }
+ 
+    private static class ReadToStringBypassCallable extends MasterToSlaveCallable<String, Exception> {
+        private final File file;
+ 
+        private ReadToStringBypassCallable(File file) {
+            this.file = file;
+        }
+ 
+        @Override
+        public String call() throws Exception {
+            final Class<?> readToStringClass = Class.forName("hudson.FilePath$ReadToString");
+            final Constructor<?> constructor = readToStringClass.getDeclaredConstructor(); // Used to have FilePath.class from non-static context
+            constructor.setAccessible(true);
+ 
+            //FilePath agentFilePath = new FilePath(new File("on agent lol")); // only used for the core code before fix
+ 
+            final SlaveToMasterFileCallable<?> callable = (SlaveToMasterFileCallable<?>) constructor.newInstance(); // agentFilePath
+ 
+            FilePath controllerFilePath = toFilePathOnController(new File(file, "secrets/master.key"));
+            final Object returned = controllerFilePath.act(callable);
+            return (String) returned;
+        }
+    }
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2531")
+    public void testSymlinkCheck() throws Exception {
+        final File buildDir = j.buildAndAssertSuccess(j.createFreeStyleProject()).getRootDir();
+        assertThrowsIOExceptionCausedBySecurityException(() -> invokeOnAgent(new SymlinkCreator(buildDir)));
+    }
+    private static class SymlinkCreator extends MasterToSlaveCallable<String, Exception> {
+        private final File baseDir;
+        private SymlinkCreator(File baseDir) {
+            this.baseDir = baseDir;
+        }
+ 
+        @Override
+        public String call() throws Exception {
+            toFilePathOnController(new File(baseDir, "child")).symlinkTo(baseDir.getPath() + "child2", TaskListener.NULL);
+            return null;
+        }
+    }
+ 
+    // --------
+ 
+    // --------
+ 
+    @Test
+    @Issue("SECURITY-2538") // SECURITY-2538 adjacent, confirms that reading, stating etc. is supposed to be possible for userContent
+    public void testReadUserContent() throws Exception {
+        invokeOnAgent(new UserContentReader(new File(j.jenkins.getRootDir(), "userContent")));
+    }
+    private static class UserContentReader extends MasterToSlaveCallable<String, Exception> {
+        private final File userContent;
+ 
+        private UserContentReader(File userContent) {
+            this.userContent = userContent;
… diff truncated

@@ -24,10 +24,13 @@
  
 package jenkins.security.s2m;
  
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.jvnet.hudson.test.LoggerRule.recorded;
  
 import hudson.FilePath;
 import hudson.model.Slave;
@@ -37,19 +40,25 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.lang.reflect.Field;
+import java.util.logging.Level;
 import javax.inject.Inject;
+import jenkins.SoloFilePathFilter;
 import org.jenkinsci.remoting.RoleChecker;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
  
 public class AdminFilePathFilterTest {
  
     @Rule
     public JenkinsRule r = new JenkinsRule();
  
+    @Rule
+    public LoggerRule logging = new LoggerRule().record(SoloFilePathFilter.class, Level.WARNING);
+ 
     @Inject
     AdminWhitelistRule rule;
  
@@ -186,6 +195,7 @@ private void checkSlave_can_readFile(Slave s, FilePath target) throws Exception
     
     private void checkSlave_cannot_readFile(Slave s, FilePath target) throws Exception {
         try {
+            logging.capture(10);
             s.getChannel().call(new ReadFileS2MCallable(target));
             fail("Slave should not be able to read file in " + target.getRemote());
         } catch (IOException e){
@@ -194,7 +204,9 @@ private void checkSlave_cannot_readFile(Slave s, FilePath target) throws Excepti
             SecurityException se = (SecurityException) t;
             StringWriter sw = new StringWriter();
             se.printStackTrace(new PrintWriter(sw));
-            assertTrue(sw.toString().contains("agent may not read"));
+            assertTrue(sw.toString().contains("Agent may not access a file path"));
+ 
+            assertThat(logging, recorded(containsString("Agent may not 'read' at")));
         }
     }