From 71d2ecf1a4e5303e80815eaa3935c4f2fa3d9104 Mon Sep 17 00:00:00 2001 From: Jeff Thompson Date: Mon, 11 Jan 2021 16:52:59 +0000 Subject: [PATCH] [SECURITY-1452] --- core/src/main/java/hudson/FilePath.java | 266 +++- .../hudson/model/DirectoryBrowserSupport.java | 46 +- .../src/main/java/hudson/util/DirScanner.java | 23 +- .../java/hudson/util/io/ArchiverFactory.java | 15 + .../main/java/hudson/util/io/ZipArchiver.java | 10 +- .../main/java/jenkins/util/VirtualFile.java | 256 +++- .../model/DirectoryBrowserSupport/dir.jelly | 14 +- .../java/jenkins/util/VirtualFileTest.java | 1131 ++++++++++++++++- .../model/DirectoryBrowserSupportTest.java | 250 +++- .../hudson/tasks/ArtifactArchiverTest.java | 2 +- 10 files changed, 1899 insertions(+), 114 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index e5ceab7847..ba8e7f26b4 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -83,9 +83,10 @@ import java.net.URLConnection; import java.nio.file.FileSystemException; import java.nio.file.FileSystems; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.nio.file.LinkOption; +import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermission; @@ -474,6 +475,25 @@ public final class FilePath implements SerializableOnlyOverRemoting { return archive(ArchiverFactory.ZIP, out, scanner); } + /** + * Uses the given scanner on 'this' directory to list up files and then archive it to a zip stream. + * + * @param out The OutputStream to write the zip into. + * @param scanner A DirScanner for scanning the directory and filtering its contents. + * @param verificationRoot A root or base directory for checking for any symlinks in this files parentage. + * Any symlinks between a file and root should be ignored. + * Symlinks in the parentage outside root will not be checked. + * @param noFollowLinks true if it should not follow links. + * + * @return The number of files/directories archived. + * This is only really useful to check for a situation where nothing + */ + @Restricted(NoExternalUse.class) + public int zip(OutputStream out, DirScanner scanner, String verificationRoot, boolean noFollowLinks) throws IOException, InterruptedException { + ArchiverFactory archiverFactory = noFollowLinks ? ArchiverFactory.ZIP_WTHOUT_FOLLOWING_SYMLINKS : ArchiverFactory.ZIP; + return archive(archiverFactory, out, scanner, verificationRoot, noFollowLinks); + } + /** * Archives this directory into the specified archive format, to the given {@link OutputStream}, by using * {@link DirScanner} to choose what files to include. @@ -483,23 +503,48 @@ public final class FilePath implements SerializableOnlyOverRemoting { * is archived. */ public int archive(final ArchiverFactory factory, OutputStream os, final DirScanner scanner) throws IOException, InterruptedException { + return archive(factory, os, scanner, null, false); + } + + /** + * Archives this directory into the specified archive format, to the given {@link OutputStream}, by using + * {@link DirScanner} to choose what files to include. + * + * @param factory The ArchiverFactory for creating the archive. + * @param os The OutputStream to write the zip into. + * @param verificationRoot A root or base directory for checking for any symlinks in this files parentage. + * Any symlinks between a file and root should be ignored. + * Symlinks in the parentage outside root will not be checked. + * @param noFollowLinks true if it should not follow links. + * + * @return The number of files/directories archived. + * This is only really useful to check for a situation where nothing + */ + @Restricted(NoExternalUse.class) + public int archive(final ArchiverFactory factory, OutputStream os, final DirScanner scanner, + String verificationRoot, boolean noFollowLinks) throws IOException, InterruptedException { final OutputStream out = (channel!=null)?new RemoteOutputStream(os):os; - return act(new Archive(factory, out, scanner)); + return act(new Archive(factory, out, scanner, verificationRoot, noFollowLinks)); } private class Archive extends SecureFileCallable { private final ArchiverFactory factory; private final OutputStream out; private final DirScanner scanner; - Archive(ArchiverFactory factory, OutputStream out, DirScanner scanner) { + private final String verificationRoot; + private final boolean noFollowLinks; + + Archive(ArchiverFactory factory, OutputStream out, DirScanner scanner, String verificationRoot, boolean noFollowLinks) { this.factory = factory; this.out = out; this.scanner = scanner; + this.verificationRoot = verificationRoot; + this.noFollowLinks = noFollowLinks; } @Override public Integer invoke(File f, VirtualChannel channel) throws IOException { Archiver a = factory.create(out); try { - scanner.scan(f,reading(a)); + scanner.scan(f, ignoringSymlinks(reading(a), verificationRoot, noFollowLinks)); } finally { a.close(); } @@ -691,6 +736,46 @@ public final class FilePath implements SerializableOnlyOverRemoting { } } + @Restricted(NoExternalUse.class) + public boolean hasSymlink(FilePath verificationRoot, boolean noFollowLinks) throws IOException, InterruptedException { + return act(new HasSymlink(verificationRoot == null ? null : verificationRoot.remote, noFollowLinks)); + } + private static class HasSymlink extends SecureFileCallable { + private static final long serialVersionUID = 1L; + private final String verificationRoot; + private final boolean noFollowLinks; + + public HasSymlink(String verificationRoot, boolean noFollowLinks) { + this.verificationRoot = verificationRoot; + this.noFollowLinks = noFollowLinks; + } + + public Boolean invoke(File f, VirtualChannel channel) throws IOException { + return isSymlink(f, verificationRoot, noFollowLinks); + } + } + + @Restricted(NoExternalUse.class) + public boolean containsSymlink(FilePath verificationRoot, boolean noFollowLinks) throws IOException, InterruptedException { + return !list(new SymlinkRetainingFileFilter(verificationRoot, noFollowLinks)).isEmpty(); + } + + private static class SymlinkRetainingFileFilter implements FileFilter, Serializable { + + private final String verificationRoot; + private final boolean noFollowLinks; + + public SymlinkRetainingFileFilter(FilePath verificationRoot, boolean noFollowLinks) { + this.verificationRoot = verificationRoot == null ? null : verificationRoot.remote; + this.noFollowLinks = noFollowLinks; + } + + public boolean accept(File file) { + return isSymlink(file, verificationRoot, noFollowLinks); + } + private static final long serialVersionUID = 1L; + } + /** * Creates a symlink to the specified target. * @@ -1838,6 +1923,23 @@ public final class FilePath implements SerializableOnlyOverRemoting { return list((FileFilter)null); } + /** + * List up files and directories in this directory. + * + * This is intended to allow the caller to provide {@link java.nio.file.LinkOption#NOFOLLOW_LINKS} to ignore + * symlinks. + * @param verificationRoot A root or base directory for checking for any symlinks in this files parentage. + * Any symlinks between a file and root should be ignored. + * Symlinks in the parentage outside root will not be checked. + * @param noFollowLinks true if it should not follow links. + * @return Direct children of this directory. + */ + @Restricted(NoExternalUse.class) + @NonNull + public List list(FilePath verificationRoot, boolean noFollowLinks) throws IOException, InterruptedException { + return list(new SymlinkDiscardingFileFilter(verificationRoot, noFollowLinks)); + } + /** * List up subdirectories. * @@ -1982,25 +2084,112 @@ public final class FilePath implements SerializableOnlyOverRemoting { * Reads this file. */ public InputStream read() throws IOException, InterruptedException { + return read(null, false); + } + + @Restricted(NoExternalUse.class) + public InputStream read(FilePath rootPath, boolean noFollowLinks) throws IOException, InterruptedException { + String rootPathString = rootPath == null ? null : rootPath.remote; if(channel==null) { - return Files.newInputStream(fileToPath(reading(new File(remote)))); + File file = reading(new File(remote)); + InputStream inputStream = newInputStreamDenyingSymlinkAsNeeded(file, rootPathString, noFollowLinks); + return inputStream; } final Pipe p = Pipe.createRemoteToLocal(); - actAsync(new Read(p)); + actAsync(new Read(p, rootPathString, noFollowLinks)); return p.getIn(); } + + @Restricted(NoExternalUse.class) + public static InputStream newInputStreamDenyingSymlinkAsNeeded(File file, String verificationRoot, boolean noFollowLinks) throws IOException { + InputStream inputStream = null; + try { + denySymlink(file, verificationRoot, noFollowLinks); + inputStream = noFollowLinks ? Files.newInputStream(fileToPath(file), LinkOption.NOFOLLOW_LINKS) : Files.newInputStream(fileToPath(file)); + denySymlink(file, verificationRoot, noFollowLinks); + } catch (IOException ioe) { + if (inputStream != null) { + inputStream.close(); + } + throw ioe; + } + return inputStream; + } + + private static void denySymlink(File file, String root, boolean noFollowLinks) throws IOException { + /* This should be checked right before the file is opened or otherwise traversed. + If at all possible, it should also be checked immediately afterwards. + This narrows any possible race conditions that may exist in weird situations, + platforms, or implementations. + newInputStreamDenyingSymlinkAsNeeded(...) demonstrates how this would be done. + + This is useful for preventing symlink following on systems that don't support + LinkOption.NOFOLLOW_LINK. Notable among those is AIX. It is also important for + prohibiting Windows Junctions, which are not considered symlinks by the + Files.newInputStream(path, LinkOption.NOFOLLOW_LINKS) implementation. + */ + + if (isSymlink(file, root, noFollowLinks)) { + throw new IOException("Symlinks are prohibited."); + } + } + + @Restricted(NoExternalUse.class) + public static boolean isSymlink(File file, String root, boolean noFollowLinks) { + if (noFollowLinks) { + if (Util.isSymlink(file.toPath())) { + return true; + } + + return isFileAncestorSymlink(file, root); + } + return false; + } + + /** + * Determines whether an ancestor of this file is a symlink, between the specified + * file and the root path. Ancestors further up the tree are not considered. + * @param file The base file for the beginning of the search. + * @param root The root path for ending the search. + * @return True if there is a symlink within the domain. False otherwise. + */ + private static boolean isFileAncestorSymlink(File file, String root) { + if (root != null) { + Path rootPath = Paths.get(root); + Path currPath = file.toPath(); + try { + while (!getRealPath(currPath).equals(getRealPath(rootPath))) { + if (Util.isSymlink(currPath)) { + return true; + } + currPath = currPath.getParent(); + if (currPath == null) { + return false; + } + } + } catch (IOException ioe) { + return false; + } + } + return false; + } + private class Read extends SecureFileCallable { private static final long serialVersionUID = 1L; private final Pipe p; - Read(Pipe p) { + private String verificationRoot; + private boolean noFollowLinks; + + Read(Pipe p, String verificationRoot, boolean noFollowLinks) { this.p = p; + this.verificationRoot = verificationRoot; + this.noFollowLinks = noFollowLinks; } @Override public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { - try (InputStream fis = Files.newInputStream(fileToPath(reading(f))); - OutputStream out = p.getOut()) { + try (InputStream fis = newInputStreamDenyingSymlinkAsNeeded(reading(f), verificationRoot, noFollowLinks); OutputStream out = p.getOut()) { org.apache.commons.io.IOUtils.copy(fis, out); } catch (Exception x) { p.error(x); @@ -3249,6 +3438,29 @@ public final class FilePath implements SerializableOnlyOverRemoting { }; } + /** + * Wraps {@link FileVisitor} to ignore symlinks. + */ + @Restricted(NoExternalUse.class) + public static FileVisitor ignoringSymlinks(final FileVisitor v, String verificationRoot, boolean noFollowLinks) { + if (noFollowLinks) { + return new FileVisitor() { + @Override + public void visit(File f, String relativePath) throws IOException { + if (verificationRoot == null || !FilePath.isSymlink(f, verificationRoot, noFollowLinks)) { + v.visit(f, relativePath); + } + } + + @Override + public boolean understandsSymlink() { + return false; + } + }; + } + return v; + } + /** * Pass through 'f' after ensuring that we can read that file. */ @@ -3429,5 +3641,41 @@ public final class FilePath implements SerializableOnlyOverRemoting { } } + private static Path getRealPath(Path path) throws IOException { + return Functions.isWindows() ? windowsToRealPath(path) : path.toRealPath(); + } + + private static @NonNull Path windowsToRealPath(@NonNull Path path) throws IOException { + try { + return path.toRealPath(); + } + catch (IOException e) { + if (LOGGER.isLoggable(Level.FINE)) { + LOGGER.log(Level.FINE, String.format("relaxedToRealPath cannot use the regular toRealPath on %s, trying with toRealPath(LinkOption.NOFOLLOW_LINKS)", path), e); + } + } + + // that's required for specific environment like Windows Server 2016, running MSFT docker + // where the root is a + return path.toRealPath(LinkOption.NOFOLLOW_LINKS); + } + private static final SoloFilePathFilter UNRESTRICTED = SoloFilePathFilter.wrap(FilePathFilter.UNRESTRICTED); + + private static class SymlinkDiscardingFileFilter implements FileFilter, Serializable { + + private final String verificationRoot; + private final boolean noFollowLinks; + + public SymlinkDiscardingFileFilter(FilePath verificationRoot, boolean noFollowLinks) { + this.verificationRoot = verificationRoot == null ? null : verificationRoot.remote; + this.noFollowLinks = noFollowLinks; + } + + public boolean accept(File file) { + return !isSymlink(file, verificationRoot, noFollowLinks); + } + private static final long serialVersionUID = 1L; + } + } diff --git a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java index b31795eff6..0a03b25040 100644 --- a/core/src/main/java/hudson/model/DirectoryBrowserSupport.java +++ b/core/src/main/java/hudson/model/DirectoryBrowserSupport.java @@ -61,6 +61,7 @@ import jenkins.util.VirtualFile; import org.apache.commons.io.IOUtils; import org.apache.tools.zip.ZipEntry; import org.apache.tools.zip.ZipOutputStream; +import org.apache.commons.lang.StringUtils; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.HttpResponse; @@ -240,25 +241,23 @@ public final class DirectoryBrowserSupport implements HttpResponse { String base = _base.toString(); String rest = _rest.toString(); - if(!ALLOW_SYMLINK_ESCAPE && (root.supportIsDescendant() && !root.isDescendant(base))){ - LOGGER.log(Level.WARNING, "Trying to access a file outside of the directory, target: "+ base); - rsp.sendError(HttpServletResponse.SC_FORBIDDEN, "Trying to access a file outside of the directory, target: " + base); - return; - } - // this is the base file/directory VirtualFile baseFile = base.isEmpty() ? root : root.child(base); + if (baseFile.hasSymlink(getNoFollowLinks())) { + rsp.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } if(baseFile.isDirectory()) { if(zip) { rsp.setContentType("application/zip"); - zip(rsp, root, baseFile, rest); + baseFile.zip(rsp.getOutputStream(), StringUtils.isBlank(rest) ? "**" : rest, null, true, getNoFollowLinks()); return; } if (plain) { rsp.setContentType("text/plain;charset=UTF-8"); try (OutputStream os = rsp.getOutputStream()) { - for (VirtualFile kid : baseFile.list()) { + for (VirtualFile kid : baseFile.list(getNoFollowLinks())) { os.write(kid.getName().getBytes(StandardCharsets.UTF_8)); if (kid.isDirectory()) { os.write('/'); @@ -281,29 +280,30 @@ public final class DirectoryBrowserSupport implements HttpResponse { List> glob = null; boolean patternUsed = rest.length() > 0; + boolean containsSymlink = false; if(patternUsed) { // the rest is Ant glob pattern glob = patternScan(baseFile, rest, createBackRef(restSize)); } else if(serveDirIndex) { // serve directory index - glob = baseFile.run(new BuildChildPaths(baseFile, req.getLocale())); + glob = baseFile.run(new BuildChildPaths(root, baseFile, req.getLocale())); + containsSymlink = baseFile.containsSymLinkChild(getNoFollowLinks()); } if(glob!=null) { - List> filteredGlob = keepReadabilityOnlyOnDescendants(baseFile, patternUsed, glob); - // serve glob req.setAttribute("it", this); List parentPaths = buildParentPath(base,restSize); req.setAttribute("parentPath",parentPaths); req.setAttribute("backPath", createBackRef(restSize)); req.setAttribute("topPath", createBackRef(parentPaths.size()+restSize)); - req.setAttribute("files", filteredGlob); + req.setAttribute("files", glob); req.setAttribute("icon", icon); req.setAttribute("path", path); req.setAttribute("pattern",rest); req.setAttribute("dir", baseFile); + req.setAttribute("showSymlinkWarning", containsSymlink); if (ResourceDomainConfiguration.isResourceRequest(req)) { req.getView(this, "plaindir.jelly").forward(req, rsp); } else { @@ -346,12 +346,19 @@ public final class DirectoryBrowserSupport implements HttpResponse { if(LOGGER.isLoggable(Level.FINE)) LOGGER.fine("Serving "+baseFile+" with lastModified=" + lastModified + ", length=" + length); + InputStream in; + try { + in = baseFile.open(getNoFollowLinks()); + } catch (IOException ioe) { + rsp.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } if (view) { // for binary files, provide the file name for download rsp.setHeader("Content-Disposition", "inline; filename=" + baseFile.getName()); // pseudo file name to let the Stapler set text/plain - rsp.serveFile(req, baseFile.open(), lastModified, -1, length, "plain.txt"); + rsp.serveFile(req, in, lastModified, -1, length, "plain.txt"); } else { if (resourceToken != null) { // redirect to second domain @@ -675,9 +682,11 @@ public final class DirectoryBrowserSupport implements HttpResponse { } private static final class BuildChildPaths extends MasterToSlaveCallable>,IOException> { + private VirtualFile root; private final VirtualFile cur; private final Locale locale; - BuildChildPaths(VirtualFile cur, Locale locale) { + BuildChildPaths(VirtualFile root, VirtualFile cur, Locale locale) { + this.root = root; this.cur = cur; this.locale = locale; } @@ -693,7 +702,7 @@ public final class DirectoryBrowserSupport implements HttpResponse { private static List> buildChildPaths(VirtualFile cur, Locale locale) throws IOException { List> r = new ArrayList<>(); - VirtualFile[] files = cur.list(); + VirtualFile[] files = cur.list(getNoFollowLinks()); Arrays.sort(files,new FileComparator(locale)); for( VirtualFile f : files ) { @@ -708,7 +717,7 @@ public final class DirectoryBrowserSupport implements HttpResponse { while(true) { // files that don't start with '.' qualify for 'meaningful files', nor SCM related files List sub = new ArrayList<>(); - for (VirtualFile vf : f.list()) { + for (VirtualFile vf : f.list(getNoFollowLinks())) { String name = vf.getName(); if (!name.startsWith(".") && !name.equals("CVS") && !name.equals(".svn")) { sub.add(vf); @@ -733,7 +742,7 @@ public final class DirectoryBrowserSupport implements HttpResponse { * @param baseRef String like "../../../" that cancels the 'rest' portion. Can be "./" */ private static List> patternScan(VirtualFile baseDir, String pattern, String baseRef) throws IOException { - Collection files = baseDir.list(pattern, null, /* TODO what is the user expectation? */true); + Collection files = baseDir.list(pattern, null, /* TODO what is the user expectation? */true, getNoFollowLinks()); if (!files.isEmpty()) { List> r = new ArrayList<>(files.size()); @@ -776,6 +785,9 @@ public final class DirectoryBrowserSupport implements HttpResponse { pathList.add(path); } + private static boolean getNoFollowLinks() { + return !ALLOW_SYMLINK_ESCAPE; + } private static final Logger LOGGER = Logger.getLogger(DirectoryBrowserSupport.class.getName()); diff --git a/core/src/main/java/hudson/util/DirScanner.java b/core/src/main/java/hudson/util/DirScanner.java index 837b560c1e..b0679f567d 100644 --- a/core/src/main/java/hudson/util/DirScanner.java +++ b/core/src/main/java/hudson/util/DirScanner.java @@ -6,6 +6,8 @@ import org.apache.tools.ant.BuildException; import org.apache.tools.ant.DirectoryScanner; import org.apache.tools.ant.types.FileSet; import org.apache.tools.ant.types.selectors.FileSelector; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import java.io.File; import java.io.FileFilter; @@ -103,15 +105,25 @@ public abstract class DirScanner implements Serializable { private final String includes, excludes; private boolean useDefaultExcludes = true; + private final boolean followSymlinks; public Glob(String includes, String excludes) { - this.includes = includes; - this.excludes = excludes; + this(includes, excludes, true, true); } public Glob(String includes, String excludes, boolean useDefaultExcludes) { - this(includes, excludes); + this(includes, excludes, useDefaultExcludes, true); + } + + /** + * @since TODO + */ + @Restricted(NoExternalUse.class) + public Glob(String includes, String excludes, boolean useDefaultExcludes, boolean followSymlinks) { + this.includes = includes; + this.excludes = excludes; this.useDefaultExcludes = useDefaultExcludes; + this.followSymlinks = followSymlinks; } public void scan(File dir, FileVisitor visitor) throws IOException { @@ -122,14 +134,11 @@ public abstract class DirScanner implements Serializable { } FileSet fs = Util.createFileSet(dir,includes,excludes); + fs.setFollowSymlinks(followSymlinks); fs.setDefaultexcludes(useDefaultExcludes); - fs.appendSelector(new DescendantFileSelector(fs.getDir())); - if(dir.exists()) { DirectoryScanner ds = fs.getDirectoryScanner(new org.apache.tools.ant.Project()); - // due to the DescendantFileSelector usage, - // the includedFiles are only the ones that are descendant for( String f : ds.getIncludedFiles()) { File file = new File(dir, f); scanSingle(file, f, visitor); diff --git a/core/src/main/java/hudson/util/io/ArchiverFactory.java b/core/src/main/java/hudson/util/io/ArchiverFactory.java index 9a4fde46c3..75c7748a5f 100644 --- a/core/src/main/java/hudson/util/io/ArchiverFactory.java +++ b/core/src/main/java/hudson/util/io/ArchiverFactory.java @@ -25,6 +25,8 @@ package hudson.util.io; import hudson.FilePath.TarCompression; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import java.io.IOException; import java.io.OutputStream; @@ -57,6 +59,11 @@ public abstract class ArchiverFactory implements Serializable { */ public static ArchiverFactory ZIP = new ZipArchiverFactory(); + /** + * Zip format, without following symlinks. + */ + @Restricted(NoExternalUse.class) + public static ArchiverFactory ZIP_WTHOUT_FOLLOWING_SYMLINKS = new ZipWithoutSymLinksArchiverFactory(); private static final class TarArchiverFactory extends ArchiverFactory { @@ -81,5 +88,13 @@ public abstract class ArchiverFactory implements Serializable { private static final long serialVersionUID = 1L; } + private static final class ZipWithoutSymLinksArchiverFactory extends ArchiverFactory { + public Archiver create(OutputStream out) { + return new ZipArchiver(out, true); + } + + private static final long serialVersionUID = 1L; + } + private static final long serialVersionUID = 1L; } diff --git a/core/src/main/java/hudson/util/io/ZipArchiver.java b/core/src/main/java/hudson/util/io/ZipArchiver.java index 1ed1f04e3c..9c0ce62a9d 100644 --- a/core/src/main/java/hudson/util/io/ZipArchiver.java +++ b/core/src/main/java/hudson/util/io/ZipArchiver.java @@ -36,6 +36,8 @@ import org.apache.tools.zip.ZipOutputStream; import java.io.File; import java.io.IOException; import java.io.OutputStream; +import java.nio.file.LinkOption; +import java.nio.file.OpenOption; /** * {@link FileVisitor} that creates a zip archive. @@ -45,9 +47,15 @@ import java.io.OutputStream; final class ZipArchiver extends Archiver { private final byte[] buf = new byte[8192]; private final ZipOutputStream zip; + private final OpenOption[] openOptions; ZipArchiver(OutputStream out) { + this(out, false); + } + + ZipArchiver(OutputStream out, boolean failOnSymLink) { zip = new ZipOutputStream(out); + openOptions = failOnSymLink ? new LinkOption[]{LinkOption.NOFOLLOW_LINKS} : new OpenOption[0]; zip.setEncoding(System.getProperty("file.encoding")); zip.setUseZip64(Zip64Mode.AsNeeded); } @@ -73,7 +81,7 @@ final class ZipArchiver extends Archiver { if (mode!=-1) fileZipEntry.setUnixMode(mode); fileZipEntry.setTime(f.lastModified()); zip.putNextEntry(fileZipEntry); - try (InputStream in = Files.newInputStream(f.toPath())) { + try (InputStream in = Files.newInputStream(f.toPath(), openOptions)) { int len; while((len=in.read(buf))>=0) zip.write(buf,0,len); diff --git a/core/src/main/java/jenkins/util/VirtualFile.java b/core/src/main/java/jenkins/util/VirtualFile.java index e5f6efd78d..8b16775ea4 100644 --- a/core/src/main/java/jenkins/util/VirtualFile.java +++ b/core/src/main/java/jenkins/util/VirtualFile.java @@ -39,6 +39,7 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.io.Serializable; import java.net.URI; import java.net.URL; @@ -57,6 +58,8 @@ import java.util.stream.Collectors; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.util.io.Archiver; +import hudson.util.io.ArchiverFactory; import jenkins.MasterToSlaveFileCallable; import jenkins.model.ArtifactManager; import jenkins.security.MasterToSlaveCallable; @@ -173,10 +176,44 @@ public abstract class VirtualFile implements Comparable, Serializab */ public abstract @NonNull VirtualFile[] list() throws IOException; + /** + * Lists children of this directory. Only one level deep. + * + * This is intended to allow the caller to provide {@link java.nio.file.LinkOption#NOFOLLOW_LINKS} to ignore + * symlinks. However, this cannot be enforced. The base implementation here in VirtualFile ignores the openOptions. + * Some VirtualFile subclasses may not be able to provide + * an implementation in which NOFOLLOW_LINKS is used or makes sense. Implementations are free + * to ignore openOptions. Some subclasses of VirtualFile may not have a concept of symlinks. + * @param noFollowLinks if true then do not follow links. + * @return a list of children (files and subdirectories); empty for a file or nonexistent directory + * @throws IOException if it could not be opened + */ + @Restricted(NoExternalUse.class) + public @NonNull VirtualFile[] list(boolean noFollowLinks) throws IOException { + return list(); + } + @Restricted(NoExternalUse.class) public boolean supportsQuickRecursiveListing() { return false; } + + /** + * Determines when a VirtualFile has a recognized symlink. + * A recognized symlink can be the file itself or any containing directory between + * it and the optional root directory. If there is no provided root directory then + * only the file itself is considered. + * + * This base implementation ignores the existence of symlinks. + * @param noFollowLinks if true, then do not follow links. + * @return True if the file is a symlink or is referenced within a containing symlink + * directory before reaching the root directory. + * @throws IOException If there is a problem accessing the file. + */ + @Restricted(NoExternalUse.class) + public boolean hasSymlink(boolean noFollowLinks) throws IOException { + return false; + } /** * Lists only the children that are descendant of the root directory (not necessarily the current VirtualFile). @@ -218,6 +255,30 @@ public abstract class VirtualFile implements Comparable, Serializab * @since 2.118 */ public @NonNull Collection list(@NonNull String includes, @CheckForNull String excludes, boolean useDefaultExcludes) throws IOException { + return list(includes, excludes, useDefaultExcludes, false); + } + + /** + * Lists recursive files of this directory with pattern matching. + * + *

The default implementation calls {@link #list()} recursively inside {@link #run} and applies filtering to the result. + * Implementations may wish to override this more efficiently. + + * This method allows the user to specify that symlinks should not be followed by passing + * noFollowLinks as true. However, some implementations may not be able to reliably + * prevent link following. The base implementation here in VirtualFile ignores this parameter. + * @param includes comma-separated Ant-style globs as per {@link Util#createFileSet(File, String, String)} using {@code /} as a path separator; + * the empty string means no matches (use {@link SelectorUtils#DEEP_TREE_MATCH} if you want to match everything except some excludes) + * @param excludes optional excludes in similar format to {@code includes} + * @param useDefaultExcludes as per {@link AbstractFileSet#setDefaultexcludes} + * @param noFollowLinks if true then do not follow links. + * @return a list of {@code /}-separated relative names of children (files directly inside or in subdirectories) + * @throws IOException if this is not a directory, or listing was not possible for some other reason + * @since TODO + */ + @Restricted(NoExternalUse.class) + public @NonNull Collection list(@NonNull String includes, @CheckForNull String excludes, boolean useDefaultExcludes, + boolean noFollowLinks) throws IOException { Collection r = run(new CollectFiles(this)); List includePatterns = patterns(includes); List excludePatterns = patterns(excludes); @@ -231,6 +292,12 @@ public abstract class VirtualFile implements Comparable, Serializab return includePatterns.stream().anyMatch(patt -> patt.matchPath(path, true)) && excludePatterns.stream().noneMatch(patt -> patt.matchPath(path, true)); }).collect(Collectors.toSet()); } + + @Restricted(NoExternalUse.class) + public boolean containsSymLinkChild(boolean noFollowLinks) throws IOException { + return false; + } + private static final class CollectFiles extends MasterToSlaveCallable, IOException> { private static final long serialVersionUID = 1; private final VirtualFile root; @@ -266,6 +333,15 @@ public abstract class VirtualFile implements Comparable, Serializab return r; } + /** + * @since TODO + */ + @Restricted(NoExternalUse.class) + public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, + boolean noFollowLinks) throws IOException, UnsupportedOperationException { + throw new UnsupportedOperationException("Not implemented."); + } + /** * Obtains a child file. * @param name a relative path, possibly including {@code /} (but not {@code ..}) @@ -312,6 +388,18 @@ public abstract class VirtualFile implements Comparable, Serializab */ public abstract InputStream open() throws IOException; + /** + * Opens an input stream on the file so its contents can be read. + * + * @param noFollowLinks if true do not follow links. + * @return an open stream + * @throws IOException if it could not be opened + */ + @Restricted(NoExternalUse.class) + public InputStream open(boolean noFollowLinks) throws IOException { + return open(); + } + /** * Does case-insensitive comparison. * {@inheritDoc} @@ -470,6 +558,20 @@ public abstract class VirtualFile implements Comparable, Serializab return vfs; } + @NonNull + @Override + public VirtualFile[] list(boolean noFollowLinks) throws IOException { + String rootPath = determineRootPath(); + File[] kids = f.listFiles(); + List contents = new ArrayList<>(kids.length); + for (File child : kids) { + if (!FilePath.isSymlink(child, rootPath, noFollowLinks)) { + contents.add(new FileVF(child, root)); + } + } + return contents.toArray(new VirtualFile[0]); + } + @Override public boolean supportsQuickRecursiveListing() { return true; } @@ -499,6 +601,32 @@ public abstract class VirtualFile implements Comparable, Serializab } return new Scanner(includes, excludes, useDefaultExcludes).invoke(f, null); } + + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes, + boolean noFollowLinks) throws IOException { + String rootPath = determineRootPath(); + return new Scanner(includes, excludes, useDefaultExcludes, rootPath, noFollowLinks).invoke(f, null); + } + + @Override + public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, + boolean noFollowLinks) throws IOException { + String rootPath = determineRootPath(); + DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, !noFollowLinks); + ArchiverFactory archiverFactory = noFollowLinks ? ArchiverFactory.ZIP_WTHOUT_FOLLOWING_SYMLINKS : ArchiverFactory.ZIP; + try (Archiver archiver = archiverFactory.create(outputStream)) { + globScanner.scan(f, FilePath.ignoringSymlinks(archiver, rootPath, noFollowLinks)); + return archiver.countEntries(); + } + } + + @Override + public boolean hasSymlink(boolean noFollowLinks) throws IOException { + String rootPath = determineRootPath(); + return FilePath.isSymlink(f, rootPath, noFollowLinks); + } + @Override public VirtualFile child(String name) { return new FileVF(new File(f, name), root); } @@ -536,6 +664,28 @@ public abstract class VirtualFile implements Comparable, Serializab throw new IOException(e); } } + @Override + public InputStream open(boolean noFollowLinks) throws IOException { + String rootPath = determineRootPath(); + InputStream inputStream = FilePath.newInputStreamDenyingSymlinkAsNeeded(f, rootPath, noFollowLinks); + return inputStream; + } + + @Override + public boolean containsSymLinkChild(boolean noFollowLinks) { + String rootPath = determineRootPath(); + File[] kids = f.listFiles(); + for (File child : kids) { + if (FilePath.isSymlink(child, rootPath, noFollowLinks)) { + return true; + } + } + return false; + } + + private String determineRootPath() { + return root == null ? null : root.getPath(); + } private boolean isIllegalSymlink() { try { @@ -680,11 +830,44 @@ public abstract class VirtualFile implements Comparable, Serializab @Override public VirtualFile[] list() throws IOException { try { List kids = f.list(); - VirtualFile[] vfs = new VirtualFile[kids.size()]; - for (int i = 0; i < vfs.length; i++) { - vfs[i] = new FilePathVF(kids.get(i), this.root); - } - return vfs; + return convertChildrenToVirtualFile(kids); + } catch (InterruptedException x) { + throw new IOException(x); + } + } + + private VirtualFile[] convertChildrenToVirtualFile(List kids) { + VirtualFile[] vfs = new VirtualFile[kids.size()]; + for (int i = 0; i < vfs.length; i++) { + vfs[i] = new FilePathVF(kids.get(i), this.root); + } + return vfs; + } + + @NonNull + @Override + public VirtualFile[] list(boolean noFollowLinks) throws IOException { + try { + List kids = f.list(root, noFollowLinks); + return convertChildrenToVirtualFile(kids); + } catch (InterruptedException x) { + throw new IOException(x); + } + } + + @Override + public boolean containsSymLinkChild(boolean noFollowLinks) throws IOException { + try { + return f.containsSymlink(root, noFollowLinks); + } catch (InterruptedException x) { + throw new IOException(x); + } + } + + @Override + public boolean hasSymlink(boolean noFollowLinks) throws IOException { + try { + return f.hasSymlink(root, noFollowLinks); } catch (InterruptedException x) { throw new IOException(x); } @@ -716,13 +899,37 @@ public abstract class VirtualFile implements Comparable, Serializab } } - @Override public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { - try { - return f.act(new Scanner(includes, excludes, useDefaultExcludes)); - } catch (InterruptedException x) { - throw new IOException(x); + @Override public Collection list(String includes, String excludes, boolean useDefaultExcludes) throws IOException { + try { + return f.act(new Scanner(includes, excludes, useDefaultExcludes)); + } catch (InterruptedException x) { + throw new IOException(x); + } } - } + + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes, + boolean noFollowLinks) throws IOException { + try { + String rootPath = root == null ? null : root.getRemote(); + return f.act(new Scanner(includes, excludes, useDefaultExcludes, rootPath, noFollowLinks)); + } catch (InterruptedException x) { + throw new IOException(x); + } + } + + @Override + public int zip(OutputStream outputStream, String includes, String excludes, boolean useDefaultExcludes, + boolean noFollowLinks) throws IOException { + try { + String rootPath = root == null ? null : root.getRemote(); + DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, !noFollowLinks); + return f.zip(outputStream, globScanner, rootPath, noFollowLinks); + } catch (InterruptedException x) { + throw new IOException(x); + } + } + @Override public VirtualFile child(String name) { return new FilePathVF(f.child(name), this.root); } @@ -761,6 +968,13 @@ public abstract class VirtualFile implements Comparable, Serializab throw new IOException(x); } } + @Override public InputStream open(boolean noFollowLinks) throws IOException { + try { + return f.read(root == null ? null : root, noFollowLinks); + } catch (InterruptedException x) { + throw new IOException(x); + } + } @Override public V run(Callable callable) throws IOException { try { return f.act(callable); @@ -841,22 +1055,34 @@ public abstract class VirtualFile implements Comparable, Serializab private static final class Scanner extends MasterToSlaveFileCallable> { private final String includes, excludes; private final boolean useDefaultExcludes; - Scanner(String includes, String excludes, boolean useDefaultExcludes) { + private final String verificationRoot; + private final boolean noFollowLinks; + + Scanner(String includes, String excludes, boolean useDefaultExcludes, String verificationRoot, boolean noFollowLinks) { this.includes = includes; this.excludes = excludes; this.useDefaultExcludes = useDefaultExcludes; + this.verificationRoot = verificationRoot; + this.noFollowLinks = noFollowLinks; } + + public Scanner(String includes, String excludes, boolean useDefaultExcludes) { + this(includes, excludes, useDefaultExcludes, null, false); + } + @Override public List invoke(File f, VirtualChannel channel) throws IOException { if (includes.isEmpty()) { // see Glob class Javadoc, and list(String, String, boolean) note return Collections.emptyList(); } final List paths = new ArrayList<>(); - new DirScanner.Glob(includes, excludes, useDefaultExcludes).scan(f, new FileVisitor() { + FileVisitor listing = new FileVisitor() { @Override - public void visit(File f, String relativePath) throws IOException { + public void visit(File f, String relativePath) { paths.add(relativePath.replace('\\', '/')); } - }); + }; + DirScanner.Glob globScanner = new DirScanner.Glob(includes, excludes, useDefaultExcludes, !noFollowLinks); + globScanner.scan(f, FilePath.ignoringSymlinks(listing, verificationRoot, noFollowLinks)); return paths; } diff --git a/core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly b/core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly index 0b12450d52..875432a281 100644 --- a/core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly +++ b/core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly @@ -45,6 +45,12 @@ THE SOFTWARE. ${%No files in directory} + +

+ + ${%Symlinks are hidden} +

+ @@ -105,7 +111,13 @@ THE SOFTWARE. -
+ + +

+ + ${%Symlinks are hidden} +

+ diff --git a/core/src/test/java/jenkins/util/VirtualFileTest.java b/core/src/test/java/jenkins/util/VirtualFileTest.java index 8544c3f398..d651a438db 100644 --- a/core/src/test/java/jenkins/util/VirtualFileTest.java +++ b/core/src/test/java/jenkins/util/VirtualFileTest.java @@ -39,13 +39,22 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.jvnet.hudson.test.Issue; +import javax.annotation.Nonnull; import java.io.File; import java.io.FileNotFoundException; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.net.URI; +import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.attribute.PosixFilePermissions; +import java.util.Arrays; +import java.util.Calendar; +import java.util.Collection; +import java.util.Date; +import java.util.GregorianCalendar; +import java.util.List; import java.util.Set; import java.util.TreeSet; import java.util.stream.Collectors; @@ -186,6 +195,105 @@ public class VirtualFileTest { } } + @Test + @Issue("SECURITY-1452") + public void list_IllegalSymlink_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + File a = new File(root, "a"); + VirtualFile virtualRoot = VirtualFile.forFile(a); + VirtualFile virtualChild = virtualRoot.child("_b"); + Collection children = virtualChild.list("**", null, true); + assertThat(children, empty()); + } + + @Test + @Issue("SECURITY-1452") + public void list_Glob_NoFollowLinks_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = VirtualFile.forFile(root); + Collection children = virtualRoot.list("**", null, true, true); + assertThat(children, containsInAnyOrder( + "a/aa/aa.txt", + "a/ab/ab.txt", + "b/ba/ba.txt" + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_Glob_NoFollowLinks_FilePathVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + Collection children = virtualRoot.list("**", null, true, true); + assertThat(children, containsInAnyOrder( + "a/aa/aa.txt", + "a/ab/ab.txt", + "b/ba/ba.txt" + )); + } + + @Test + @Issue("SECURITY-1452") + public void zip_NoFollowLinks_FilePathVF() throws Exception { + File zipFile = new File(tmp.getRoot(), "output.zip"); + File root = tmp.getRoot(); + File source = new File(root, "source"); + prepareFileStructureForIsDescendant(source); + + VirtualFile sourcePath = VirtualFile.forFilePath(new FilePath(source)); + try (FileOutputStream outputStream = new FileOutputStream(zipFile)) { + sourcePath.zip( outputStream,"**", null, true, true); + } + FilePath zipPath = new FilePath(zipFile); + assertTrue(zipPath.exists()); + assertFalse(zipPath.isDirectory()); + FilePath unzipPath = new FilePath(new File(tmp.getRoot(), "unzip")); + zipPath.unzip(unzipPath); + assertTrue(unzipPath.exists()); + assertTrue(unzipPath.isDirectory()); + assertTrue(unzipPath.child("a").child("aa").child("aa.txt").exists()); + assertTrue(unzipPath.child("a").child("ab").child("ab.txt").exists()); + assertFalse(unzipPath.child("a").child("aa").child("aaa").exists()); + assertFalse(unzipPath.child("a").child("_b").exists()); + assertTrue(unzipPath.child("b").child("ba").child("ba.txt").exists()); + assertFalse(unzipPath.child("b").child("_a").exists()); + assertFalse(unzipPath.child("b").child("_aatxt").exists()); + } + + @Test + @Issue("SECURITY-1452") + public void zip_NoFollowLinks_FileVF() throws Exception { + File zipFile = new File(tmp.getRoot(), "output.zip"); + File root = tmp.getRoot(); + File source = new File(root, "source"); + prepareFileStructureForIsDescendant(source); + + VirtualFile sourcePath = VirtualFile.forFile(source); + try (FileOutputStream outputStream = new FileOutputStream(zipFile)) { + sourcePath.zip( outputStream,"**", null, true, true); + } + FilePath zipPath = new FilePath(zipFile); + assertTrue(zipPath.exists()); + assertFalse(zipPath.isDirectory()); + FilePath unzipPath = new FilePath(new File(tmp.getRoot(), "unzip")); + zipPath.unzip(unzipPath); + assertTrue(unzipPath.exists()); + assertTrue(unzipPath.isDirectory()); + assertTrue(unzipPath.child("a").child("aa").child("aa.txt").exists()); + assertTrue(unzipPath.child("a").child("ab").child("ab.txt").exists()); + assertFalse(unzipPath.child("a").child("aa").child("aaa").exists()); + assertFalse(unzipPath.child("a").child("_b").exists()); + assertTrue(unzipPath.child("b").child("ba").child("ba.txt").exists()); + assertFalse(unzipPath.child("b").child("_a").exists()); + assertFalse(unzipPath.child("b").child("_aatxt").exists()); + } + @Issue("JENKINS-26810") @Test public void readLink() throws Exception { assumeFalse("Symlinks do not work well on Windows", Functions.isWindows()); @@ -204,6 +312,344 @@ public class VirtualFileTest { } } + @Test + public void simpleList_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = VirtualFile.forFile(root); + List children = Arrays.asList(virtualRoot.list()); + assertThat(children, hasSize(2)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("a"), + VFMatcher.hasName("b") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = VirtualFile.forFile(root); + List children = Arrays.asList(virtualRoot.list(true)); + assertThat(children, hasSize(2)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("a"), + VFMatcher.hasName("b") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_FilePathVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + List children = Arrays.asList(virtualRoot.list(true)); + assertThat(children, hasSize(2)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("a"), + VFMatcher.hasName("b") + )); + } + + @Test + @Issue("SECURITY-1452") + public void simpleList_WithSymlink_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = VirtualFile.forFile(root); + VirtualFile virtualRootChildA = virtualRoot.child("a"); + List children = Arrays.asList(virtualRootChildA.list()); + assertThat(children, hasSize(3)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab"), + VFMatcher.hasName("_b") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_ExternalSymlink_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + File root = tmp.getRoot(); + String symlinkName = "symlink"; + Util.createSymlink(root, "a", symlinkName, null); + File symlinkFile = new File(root, symlinkName); + VirtualFile virtualRootSymlink = VirtualFile.forFile(symlinkFile); + List children = Arrays.asList(virtualRootSymlink.list(true)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_ExternalSymlink_FilePathVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + File root = tmp.getRoot(); + String symlinkName = "symlink"; + Util.createSymlink(root, "a", symlinkName, null); + File symlinkFile = new File(root, symlinkName); + VirtualFile virtualRootSymlink = VirtualFile.forFilePath(new FilePath(symlinkFile)); + List children = Arrays.asList(virtualRootSymlink.list(true)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_Glob_NoFollowLinks_ExternalSymlink_FilePathVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + File root = tmp.getRoot(); + String symlinkName = "symlink"; + Util.createSymlink(root, "a", symlinkName, null); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + File symlinkFile = new File(root, symlinkName); + FilePath symlinkPath = new FilePath(symlinkFile); + VirtualFile symlinkVirtualPath = VirtualFile.forFilePath(symlinkPath); + VirtualFile symlinkChildVirtualPath = symlinkVirtualPath.child("aa"); + Collection children = symlinkChildVirtualPath.list("**", null, true, true); + assertThat(children, contains("aa.txt")); + } + + @Test + @Issue("SECURITY-1452") + public void list_Glob_NoFollowLinks_ExternalSymlink_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + File root = tmp.getRoot(); + String symlinkName = "symlink"; + Util.createSymlink(root, "a", symlinkName, null); + File symlinkFile = new File(root, symlinkName); + VirtualFile symlinkVirtualFile = VirtualFile.forFile(symlinkFile); + VirtualFile symlinkChildVirtualFile = symlinkVirtualFile.child("aa"); + Collection children = symlinkChildVirtualFile.list("**", null, true, true); + assertThat(children, contains("aa.txt")); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_InternalSymlink_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile rootVirtualFile = VirtualFile.forFile(root); + VirtualFile virtualRootChildA = rootVirtualFile.child("a"); + List children = Arrays.asList(virtualRootChildA.list(true)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_InternalSymlink_FilePathVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + FilePath rootPath = new FilePath(root); + VirtualFile rootVirtualPath = VirtualFile.forFilePath(rootPath); + VirtualFile virtualRootChildA = rootVirtualPath.child("a"); + List children = Arrays.asList(virtualRootChildA.list(true)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_NoKids_FileVF() throws Exception { + File root = tmp.getRoot(); + FileUtils.touch(root); + VirtualFile virtualRoot = VirtualFile.forFile(root); + List children = Arrays.asList(virtualRoot.list()); + assertThat(children, empty()); + } + + @Test + @Issue("SECURITY-1452") + public void list_Glob_NoFollowLinks_NoKids_FileVF() throws Exception { + File root = tmp.getRoot(); + FileUtils.touch(root); + VirtualFile virtualRoot = VirtualFile.forFile(root); + Collection children = virtualRoot.list("**", null, true, true); + assertThat(children, empty()); + } + + @Test + @Issue("SECURITY-1452") + public void list_Glob_NoFollowLinks_NoKids_FilePathVF() throws Exception { + File root = tmp.getRoot(); + FileUtils.touch(root); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + Collection children = virtualRoot.list("**", null, true, true); + assertThat(children, empty()); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_NoKids_FilePathVF() throws Exception { + File root = tmp.getRoot(); + FileUtils.touch(root); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + List children = Arrays.asList(virtualRoot.list(false)); + assertThat(children, empty()); + } + + @Test + public void simpleList_NoKids_FileVF() throws Exception { + File root = tmp.getRoot(); + FileUtils.touch(root); + VirtualFile virtualRoot = VirtualFile.forFile(root); + List children = Arrays.asList(virtualRoot.list()); + assertThat(children, empty()); + } + + @Test + @Issue("SECURITY-1452") + public void simpleList_IllegalSymlink_FileVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + File a = new File(root, "a"); + VirtualFile virtualRoot = VirtualFile.forFile(a); + VirtualFile virtualChild = virtualRoot.child("_b"); + List children = Arrays.asList(virtualChild.list()); + assertThat(children, empty()); + } + + @Test + public void simpleList_FilePathVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + List children = Arrays.asList(virtualRoot.list()); + assertThat(children, hasSize(2)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("a"), + VFMatcher.hasName("b") + )); + } + + @Test + @Issue("SECURITY-1452") + public void simpleList_WithSymlink_FilePathVF() throws Exception { + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + VirtualFile virtualRootChildA = virtualRoot.child("a"); + List children = Arrays.asList(virtualRootChildA.list()); + assertThat(children, hasSize(3)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab"), + VFMatcher.hasName("_b") + )); + } + + @Test + public void simpleList_NoKids_FilePathVF() throws Exception { + File root = tmp.getRoot(); + FileUtils.touch(root); + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(root)); + List children = Arrays.asList(virtualRoot.list()); + assertThat(children, empty()); + } + + @Test + public void simpleList_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which has limited behavior. + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = new VirtualFileMinimalImplementation(root); + List children = Arrays.asList(virtualRoot.list()); + assertThat(children, hasSize(2)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("a"), + VFMatcher.hasName("b") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which has limited behavior. + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = new VirtualFileMinimalImplementation(root); + List children = Arrays.asList(virtualRoot.list(true)); + assertThat(children, hasSize(2)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("a"), + VFMatcher.hasName("b") + )); + } + + @Test + @Issue("SECURITY-1452") + public void simpleList_WithSymlink_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which has limited behavior. + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = new VirtualFileMinimalImplementation(root); + VirtualFile virtualRootChildA = virtualRoot.child("a"); + List children = Arrays.asList(virtualRootChildA.list()); + assertThat(children, hasSize(3)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab"), + VFMatcher.hasName("_b") + )); + } + + @Test + @Issue("SECURITY-1452") + public void list_NoFollowLinks_WithSymlink_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which generally does nothing. + prepareFileStructureForIsDescendant(tmp.getRoot()); + + File root = tmp.getRoot(); + VirtualFile virtualRoot = new VirtualFileMinimalImplementation(root); + VirtualFile virtualRootChildA = virtualRoot.child("a"); + List children = Arrays.asList(virtualRootChildA.list(true)); + assertThat(children, hasSize(3)); + assertThat(children, containsInAnyOrder( + VFMatcher.hasName("aa"), + VFMatcher.hasName("ab"), + VFMatcher.hasName("_b") + )); + } + + @Test + public void simpleList_NoKids_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which generally does nothing. + File root = tmp.getRoot(); + FileUtils.touch(root); + VirtualFile virtualRoot = new VirtualFileMinimalImplementation(root); + List children = Arrays.asList(virtualRoot.list()); + assertThat(children, empty()); + } + // root // /a // /aa @@ -218,8 +664,7 @@ public class VirtualFileTest { // /_aatxt => symlink to /root/a/aa/aa.txt // /ba // ba.txt - private void prepareFileStructureForIsDescendant() throws Exception { - File root = tmp.getRoot(); + private void prepareFileStructureForIsDescendant(File root) throws Exception { File a = new File(root, "a"); File aa = new File(a, "aa"); File aaa = new File(aa, "aaa"); @@ -253,7 +698,7 @@ public class VirtualFileTest { @Issue("SECURITY-904") @Test public void forFile_isDescendant() throws Exception { - this.prepareFileStructureForIsDescendant(); + this.prepareFileStructureForIsDescendant(tmp.getRoot()); File root = tmp.getRoot(); File a = new File(root, "a"); @@ -269,7 +714,7 @@ public class VirtualFileTest { @Test @Issue("SECURITY-904") public void forFilePath_isDescendant() throws Exception { - this.prepareFileStructureForIsDescendant(); + this.prepareFileStructureForIsDescendant(tmp.getRoot()); File root = tmp.getRoot(); File a = new File(root, "a"); @@ -344,7 +789,7 @@ public class VirtualFileTest { @Test @Issue("JENKINS-55050") public void forFile_listOnlyDescendants_withoutIllegal() throws Exception { - this.prepareFileStructureForIsDescendant(); + this.prepareFileStructureForIsDescendant(tmp.getRoot()); File root = tmp.getRoot(); File a = new File(root, "a"); @@ -359,7 +804,7 @@ public class VirtualFileTest { @Test @Issue("SECURITY-904") public void forFilePath_listOnlyDescendants_withoutIllegal() throws Exception { - this.prepareFileStructureForIsDescendant(); + this.prepareFileStructureForIsDescendant(tmp.getRoot()); File root = tmp.getRoot(); File a = new File(root, "a"); @@ -413,6 +858,31 @@ public class VirtualFileTest { )); } + @Test + public void forAbstractBase_listOnlyDescendants_withoutIllegal() throws Exception { + File root = tmp.getRoot(); + FileUtils.touch(new File(root, "a")); + FileUtils.touch(new File(root, "b")); + VirtualFile virtualRoot = new VirtualFileMinimalImplementation(root); + + assertThat(virtualRoot.listOnlyDescendants(), empty()); + } + + @Test + public void forAbstractBase_WithAllDescendants_listOnlyDescendants_withoutIllegal() throws Exception { + File root = tmp.getRoot(); + FileUtils.touch(new File(root, "a")); + FileUtils.touch(new File(root, "b")); + VirtualFile virtualRoot = new VirtualFileMinimalImplementationWithDescendants(root); + + List descendants = virtualRoot.listOnlyDescendants(); + assertThat(descendants, hasSize(2)); + assertThat(descendants, containsInAnyOrder( + VFMatcher.hasName("a"), + VFMatcher.hasName("b") + )); + } + private abstract static class VFMatcher extends TypeSafeMatcher { private final String description; @@ -432,4 +902,653 @@ public class VirtualFileTest { }; } } + + @Test + public void testGetParent_FileVF() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child = "child"; + File childFile = new File(parentFile, child); + VirtualFile vf = VirtualFile.forFile(childFile); + assertThat(vf.getParent().getName(), is(parentFolder)); + } + + @Test + public void testGetUri_FileVF() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child = "child"; + File childFile = new File(parentFile, child); + VirtualFile vf = VirtualFile.forFile(childFile); + URI uri = vf.toURI(); + assertThat(uri.getScheme(), is("file")); + assertThat(uri.getPath(), endsWith(parentFolder + "/" + child)); + } + + @Test + @Issue("SECURITY-1452") + public void testIsDirectory_IllegalSymLink_FileVF() throws IOException, InterruptedException { + String invalidSymlinkName = "invalidSymlink"; + File ws = createInvalidDirectorySymlink(invalidSymlinkName); + VirtualFile root = VirtualFile.forFile(ws); + VirtualFile invalidSymlink = root.child(invalidSymlinkName); + assertFalse(invalidSymlink.isDirectory()); + } + + @Test + @Issue("SECURITY-1452") + public void testReadLink_IllegalSymLink_FileVF() throws IOException, InterruptedException { + String invalidSymlinkName = "invalidSymlink"; + File ws = createInvalidDirectorySymlink(invalidSymlinkName); + VirtualFile root = VirtualFile.forFile(ws); + VirtualFile invalidSymlink = root.child(invalidSymlinkName); + assertThat(invalidSymlink.readLink(), nullValue()); + } + + @Test + public void testLength_FileVF() throws IOException { + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.write(new File(ws, childString), childString); + VirtualFile child = VirtualFile.forFile(ws).child(childString); + assertThat(child.length(), is((long)childString.length())); + } + + @Test + @Issue("SECURITY-1452") + public void testLength_IllegalSymLink_FileVF() throws IOException, InterruptedException { + File ws = createInvalidFileSymlink(); + VirtualFile root = VirtualFile.forFile(ws); + VirtualFile invalidSymlink = root.child("invalidSymlink"); + assertThat(invalidSymlink.length(), is(0L)); + } + + @Test + @Issue("SECURITY-1452") + public void testMode_IllegalSymLink_FileVF() throws Exception { + File ws = createInvalidFileSymlink(); + VirtualFile root = VirtualFile.forFile(ws); + VirtualFile invalidSymlink = root.child("invalidSymlink"); + assertThat(invalidSymlink.mode(), is(-1)); + } + + @Test + public void testLastModified_FileVF() throws Exception { + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.touch(new File(ws, childString)); + VirtualFile child = VirtualFile.forFile(ws).child(childString); + long earlierSystemTime = computeEarlierSystemTime(); + assertThat(child.lastModified(), greaterThan(earlierSystemTime)); + } + + @Test + public void testLastModified_IllegalSymLink_FileVF() throws Exception { + File ws = createInvalidFileSymlink(); + VirtualFile root = VirtualFile.forFile(ws); + VirtualFile invalidSymlink = root.child("invalidSymlink"); + assertThat(invalidSymlink.lastModified(), is(0L)); + } + + @Test + public void testCanRead_True_FileVF() throws Exception { + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.touch(new File(ws, childString)); + VirtualFile child = VirtualFile.forFile(ws).child(childString); + assertTrue(child.canRead()); + } + + @Test + public void testCanRead_False_FileVF() throws Exception { + File ws = tmp.newFolder("ws"); + String childString = "child"; + File childFile = new File(ws, childString); + FileUtils.touch(childFile); + childFile.setReadable(false); + VirtualFile child = VirtualFile.forFile(ws).child(childString); + // Windows ignores this setting. On Unix, setting this flag means it cannot be read. + assertEquals(Functions.isWindows(), child.canRead()); + } + + @Test + @Issue("SECURITY-1452") + public void testCanRead_IllegalSymlink_FileVF() throws Exception { + File ws = createInvalidFileSymlink(); + VirtualFile root = VirtualFile.forFile(ws); + VirtualFile invalidSymlink = root.child("invalidSymlink"); + assertFalse(invalidSymlink.canRead()); + } + + @Test + @Issue("SECURITY-1452") + public void testOpenNoFollowLinks_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which generally does nothing. + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.write(new File(ws, childString), childString); + VirtualFile child = new VirtualFileMinimalImplementation(ws).child(childString); + String fileContents = IOUtils.toString(child.open()); + assertThat(childString, is(fileContents)); + } + + @Test + @Issue("SECURITY-1452") + public void testOpenNoFollowLinks_FollowsLink_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which generally does nothing. + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.write(new File(ws, childString), childString); + String linkString = "link"; + Util.createSymlink(ws, childString, linkString, TaskListener.NULL); + + VirtualFile link = new VirtualFileMinimalImplementation(ws).child(linkString); + String fileContents = IOUtils.toString(link.open(true)); + assertThat(childString, is(fileContents)); + } + + @Test + @Issue("SECURITY-1452") + public void testOpenNoFollowLinks_NoFollowsLink_FileVF() throws Exception { + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.write(new File(ws, childString), childString); + String linkString = "link"; + Util.createSymlink(ws, childString, linkString, TaskListener.NULL); + + VirtualFile link = VirtualFile.forFile(ws).child(linkString); + try { + link.open(true); + fail("Should have not followed links."); + } catch (IOException ioe) { + // expected + } + } + + @Test + @Issue("SECURITY-1452") + public void testOpenNoFollowLinks_NoFollowsLinkInternalSymlink_FileVF() throws Exception { + File ws = tmp.newFolder("ws"); + VirtualFile rootVirtualFile = VirtualFile.forFile(tmp.getRoot()); + String symlinkName = "symlink"; + Util.createSymlink(tmp.getRoot(), ws.getName(), symlinkName, null); + String childString = "child"; + FileUtils.write(new File(ws, childString), childString); + File childThroughSymlink = new File(tmp.getRoot(), "/" + symlinkName + "/" + childString); + VirtualFile child = rootVirtualFile.child(symlinkName).child(childString); + try { + child.open(true); + fail("Should have not followed links."); + } catch (IOException ioe) { + // expected + } + } + + @Test + @Issue("SECURITY-1452") + public void testOpenNoFollowLinks_NoFollowsLinkInternalSymlink_FilePathVF() throws Exception { + File ws = tmp.newFolder("ws"); + String symlinkName = "symlink"; + Util.createSymlink(tmp.getRoot(), ws.getName(), symlinkName, null); + String childString = "child"; + FileUtils.write(new File(ws, childString), childString); + VirtualFile rootVirtualPath = VirtualFile.forFilePath(new FilePath(tmp.getRoot())); + VirtualFile childVirtualPath = rootVirtualPath.child(symlinkName).child(childString); + try { + childVirtualPath.open(true); + fail("Should have not followed links."); + } catch (IOException ioe) { + // expected + } + } + + @Test + @Issue("SECURITY-1452") + public void testOpenNoFollowLinks_NoFollowsLink_FilePathVF() throws Exception { + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.write(new File(ws, childString), childString); + String linkString = "link"; + Util.createSymlink(ws, childString, linkString, TaskListener.NULL); + + VirtualFile link = VirtualFile.forFilePath(new FilePath(ws)).child(linkString); + try { + link.open(true); + fail("Should have not followed links."); + } catch (IOException ioe) { + // expected + } + } + + @Test + public void testSupportIsDescendant_FileVF() throws Exception { + File ws = tmp.newFolder("ws"); + VirtualFile root = VirtualFile.forFile(ws); + assertTrue(root.supportIsDescendant()); + } + + @Test + public void testSupportsQuickRecursiveListing_FileVF() throws Exception { + File ws = tmp.newFolder("ws"); + VirtualFile root = VirtualFile.forFile(ws); + assertTrue(root.supportsQuickRecursiveListing()); + } + + @Test + public void testGetParent_FilePathVF() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child = "child"; + File childFile = new File(parentFile, child); + VirtualFile vf = VirtualFile.forFilePath(new FilePath(childFile)); + assertThat(vf.getParent().getName(), is(parentFolder)); + } + + @Test + public void testGetUri_FilePathVF() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child = "child"; + File childFile = new File(parentFile, child); + VirtualFile vf = VirtualFile.forFilePath(new FilePath(childFile)); + URI uri = vf.toURI(); + assertThat(uri.getScheme(), is("file")); + assertThat(uri.getPath(), endsWith(parentFolder + "/" + child)); + } + + @Test + public void testLength_FilePathVF() throws IOException { + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.write(new File(ws, childString), childString); + VirtualFile child = VirtualFile.forFilePath(new FilePath(ws)).child(childString); + assertThat(child.length(), is((long)childString.length())); + } + + @Test + public void testLastModified_FilePathVF() throws Exception { + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.touch(new File(ws, childString)); + VirtualFile child = VirtualFile.forFilePath(new FilePath(ws)).child(childString); + long earlierSystemTime = computeEarlierSystemTime(); + assertThat(child.lastModified(), greaterThan(earlierSystemTime)); + } + + @Test + public void testCanRead_True_FilePathVF() throws Exception { + File ws = tmp.newFolder("ws"); + String childString = "child"; + FileUtils.touch(new File(ws, childString)); + VirtualFile child = VirtualFile.forFilePath(new FilePath(ws)).child(childString); + assertTrue(child.canRead()); + } + + @Test + public void testCanRead_False_FilePathVF() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which generally does nothing. + File ws = tmp.newFolder("ws"); + String childString = "child"; + File childFile = new File(ws, childString); + FileUtils.touch(childFile); + childFile.setReadable(false); + VirtualFile child = VirtualFile.forFilePath(new FilePath(ws)).child(childString); + assertEquals(Functions.isWindows(), child.canRead()); + } + + @Test + public void testSupportIsDescendant_FilePathVF() throws Exception { + File ws = tmp.newFolder("ws"); + VirtualFile root = VirtualFile.forFilePath(new FilePath(ws)); + assertTrue(root.supportIsDescendant()); + } + + @Test + public void testSupportsQuickRecursiveListing_FilePathVF() throws Exception { + File ws = tmp.newFolder("ws"); + VirtualFile root = VirtualFile.forFilePath(new FilePath(ws)); + assertTrue(root.supportsQuickRecursiveListing()); + } + + @Test + public void testSupportIsDescendant_AbstractBase() { + VirtualFile root = new VirtualFileMinimalImplementation(); + assertFalse(root.supportIsDescendant()); + } + + @Test + public void testSupportsQuickRecursiveListing_AbstractBase() { + VirtualFile root = new VirtualFileMinimalImplementation(); + assertFalse(root.supportsQuickRecursiveListing()); + } + + @Test + public void testReadLink_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which generally does nothing. + VirtualFile root = new VirtualFileMinimalImplementation(); + assertThat(root.readLink(), nullValue()); + } + + @Test + public void testMode_AbstractBase() throws Exception { + // This test checks the method's behavior in the abstract base class, + // which generally does nothing. + VirtualFile root = new VirtualFileMinimalImplementation(); + assertThat(root.mode(), is(-1)); + } + + @Test + public void testIsDescendant_AbstractBase() throws Exception { + VirtualFile root = new VirtualFileMinimalImplementation(); + assertFalse(root.isDescendant("anything")); + } + + @Test + public void testExternalUrl() throws Exception { + VirtualFile root = new VirtualFileMinimalImplementation(); + assertThat(root.toExternalURL(), nullValue()); + } + + @Test + public void testToString() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child = "child"; + File childFile = new File(parentFile, child); + VirtualFile vf = new VirtualFileMinimalImplementation(childFile); + String vfString = vf.toString(); + assertThat(vfString, startsWith("file:/")); + assertThat(vfString, endsWith(child)); + } + + @Test + public void testHashCode() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child = "child"; + File childFile = new File(parentFile, child); + VirtualFile vf = new VirtualFileMinimalImplementation(childFile); + assertThat(vf.hashCode(), is(childFile.toURI().hashCode())); + } + + @Test + public void testEquals_Null() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child1 = "child1"; + File childFile1 = new File(parentFile, child1); + VirtualFile vf1 = new VirtualFileMinimalImplementation(childFile1); + VirtualFile vf2 = null; + assertFalse(vf1.equals(vf2)); + } + + @Test + public void testEquals_Different() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child1 = "child1"; + File childFile1 = new File(parentFile, child1); + VirtualFile vf1 = new VirtualFileMinimalImplementation(childFile1); + String child2 = "child2"; + File childFile2 = new File(parentFile, child2); + VirtualFile vf2 = new VirtualFileMinimalImplementation(childFile2); + assertFalse(vf1.equals(vf2)); + } + + @Test + public void testEquals_Same() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child1 = "child1"; + File childFile1 = new File(parentFile, child1); + VirtualFile vf1 = new VirtualFileMinimalImplementation(childFile1); + String child2 = child1; + File childFile2 = new File(parentFile, child2); + VirtualFile vf2 = new VirtualFileMinimalImplementation(childFile2); + assertTrue(vf1.equals(vf2)); + } + + @Test + public void testEquals_OtherType() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child1 = "child1"; + File childFile1 = new File(parentFile, child1); + VirtualFile vf1 = new VirtualFileMinimalImplementation(childFile1); + assertFalse(vf1.equals(child1)); + } + + @Test + public void testCompareTo_Same() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child1 = "child1"; + File childFile1 = new File(parentFile, child1); + VirtualFile vf1 = new VirtualFileMinimalImplementation(childFile1); + String child2 = child1; + File childFile2 = new File(parentFile, child2); + VirtualFile vf2 = new VirtualFileMinimalImplementation(childFile2); + assertThat(vf1.compareTo(vf2), is(0)); + } + + @Test + public void testCompareTo_LessThan() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child1 = "child1"; + File childFile1 = new File(parentFile, child1); + VirtualFile vf1 = new VirtualFileMinimalImplementation(childFile1); + String child2 = "child2"; + File childFile2 = new File(parentFile, child2); + VirtualFile vf2 = new VirtualFileMinimalImplementation(childFile2); + assertThat(vf1.compareTo(vf2), lessThan(0)); + } + + @Test + public void testCompareTo_GreaterThan() throws IOException { + String parentFolder = "parentFolder"; + File parentFile = tmp.newFolder(parentFolder); + String child1 = "child1"; + File childFile1 = new File(parentFile, child1); + VirtualFile vf1 = new VirtualFileMinimalImplementation(childFile1); + String child2 = "child2"; + File childFile2 = new File(parentFile, child2); + VirtualFile vf2 = new VirtualFileMinimalImplementation(childFile2); + assertThat(vf2.compareTo(vf1), greaterThan(0)); + } + + @Test + public void hasSymlink_AbstractBase() throws IOException { + // This test checks the method's behavior in the abstract base class, + // which generally does nothing. + VirtualFile virtualRoot = new VirtualFileMinimalImplementation(tmp.getRoot()); + assertFalse(virtualRoot.hasSymlink(true)); + } + + @Test + public void hasSymlink_False_FilePathVF() throws IOException { + VirtualFile virtualRoot = VirtualFile.forFilePath(new FilePath(tmp.getRoot())); + assertFalse(virtualRoot.hasSymlink(true)); + } + + @Test + public void hasSymlink_True_FilePathVF() throws IOException, InterruptedException { + FilePath rootPath = new FilePath(tmp.getRoot()); + FilePath childPath = rootPath.child("child"); + childPath.touch(0); + FilePath symlinkPath = rootPath.child("symlink"); + symlinkPath.symlinkTo(childPath.getName(), null); + VirtualFile virtualFile = VirtualFile.forFilePath(symlinkPath); + assertTrue(virtualFile.hasSymlink(true)); + } + + @Test + public void hasSymlink_False_FileVF() throws IOException { + VirtualFile virtualRoot = VirtualFile.forFile(tmp.getRoot()); + assertFalse(virtualRoot.hasSymlink(true)); + } + + @Test + public void hasSymlink_True_FileVF() throws IOException, InterruptedException { + FilePath rootPath = new FilePath(tmp.getRoot()); + FilePath childPath = rootPath.child("child"); + childPath.touch(0); + FilePath symlinkPath = rootPath.child("symlink"); + symlinkPath.symlinkTo(childPath.getName(), null); + VirtualFile virtualFile = VirtualFile.forFile(new File(symlinkPath.toURI())); + assertTrue(virtualFile.hasSymlink(true)); + } + + private File createInvalidDirectorySymlink(String invalidSymlinkName) throws IOException, InterruptedException { + File ws = tmp.newFolder("ws"); + String externalFolderName = "external"; + tmp.newFolder(externalFolderName); + Util.createSymlink(ws, "../" + externalFolderName, invalidSymlinkName, TaskListener.NULL); + return ws; + } + + private File createInvalidFileSymlink() throws IOException, InterruptedException { + File ws = tmp.newFolder("ws"); + String externalFolderName = "external"; + File externalFile = tmp.newFolder(externalFolderName); + String childString = "child"; + FileUtils.write(new File(externalFile, childString), childString); + Util.createSymlink(ws, "../" + externalFolderName, "invalidSymlink", TaskListener.NULL); + return ws; + } + + private long computeEarlierSystemTime() { + long earlierSystemTime = 0L; + if (Functions.isWindows()) { + return 0L; + } + Date date = new GregorianCalendar(2018, Calendar.JANUARY, 1).getTime(); + return date.getTime(); + } + + private static class VirtualFileMinimalImplementation extends VirtualFile { + + private File file; + private File root; + + VirtualFileMinimalImplementation() { + } + + VirtualFileMinimalImplementation(File file) { + this(file, file); + } + + VirtualFileMinimalImplementation(File file, File root) { + this.file = file; + this.root = root; + } + + @Nonnull + @Override + public String getName() { + return file.getName(); + } + + @Nonnull + @Override + public URI toURI() { + return file.toURI(); + } + + @Override + public VirtualFile getParent() { + return null; + } + + @Override + public boolean isDirectory() { + return false; + } + + @Override + public boolean isFile() { + return false; + } + + @Override + public boolean exists() { + return false; + } + + @Nonnull + @Override + public VirtualFile[] list() { + File[] kids = file.listFiles(); + if (kids == null) { + return new VirtualFile[0]; + } + VirtualFile[] vfs = new VirtualFile[kids.length]; + for (int i = 0; i < kids.length; i++) { + vfs[i] = child(kids[i], root); + } + return vfs; + } + + protected VirtualFile child(File kid, File root) { + return new VirtualFileMinimalImplementation(kid, root); + } + + @Nonnull + @Override + public VirtualFile child(@Nonnull String name) { + return child(new File(file, name), root); + } + + @Override + public long length() { + return 0; + } + + @Override + public long lastModified() { + return 0; + } + + @Override + public boolean canRead() { + return false; + } + + @Override + public InputStream open() throws IOException { + return Files.newInputStream(file.toPath()); + } + + } + + private static class VirtualFileMinimalImplementationWithDescendants extends VirtualFileMinimalImplementation { + + public VirtualFileMinimalImplementationWithDescendants(File file) { + super(file); + } + + public VirtualFileMinimalImplementationWithDescendants(File file, File root) { + super(file, root); + } + + @Override + public boolean supportIsDescendant() { + return true; + } + + @Override + public boolean isDescendant(String childRelativePath) throws IOException { + return true; + } + + @Override + protected VirtualFile child(File kid, File root) { + return new VirtualFileMinimalImplementationWithDescendants(kid, root); + } + } + } diff --git a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java index be06571c4a..c88adf0d48 100644 --- a/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java +++ b/test/src/test/java/hudson/model/DirectoryBrowserSupportTest.java @@ -173,7 +173,7 @@ public class DirectoryBrowserSupportTest { ZipFile readzip = new ZipFile(zipfile); - InputStream is = readzip.getInputStream(readzip.getEntry("archive/artifact.out")); + InputStream is = readzip.getInputStream(readzip.getEntry("artifact.out")); // ZipException in case of JENKINS-19752 assertNotEquals("Downloaded zip file must not be empty", is.read(), -1); @@ -468,19 +468,19 @@ public class DirectoryBrowserSupportTest { assertThat(workspaceContent, allOf( containsString("public1.key"), containsString("intermediateFolder"), - containsString("to_secrets1"), - containsString("to_secrets_goal1"), + not(containsString("to_secrets1")), + not(containsString("to_secrets_goal1")), not(containsString("to_secrets2")), not(containsString("to_secrets_goal2")) )); } { // to_secrets1 not reachable Page page = wc.goTo(p.getUrl() + "ws/to_secrets1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_secrets_goal1 not reachable Page page = wc.goTo(p.getUrl() + "ws/to_secrets_goal1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // intermediateFolder must be reachable (regular case) Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/", null); @@ -489,21 +489,21 @@ public class DirectoryBrowserSupportTest { assertThat(workspaceContent, allOf( not(containsString("to_secrets1")), not(containsString("to_secrets_goal1")), - containsString("to_secrets2"), - containsString("to_secrets_goal2") + not(containsString("to_secrets2")), + not(containsString("to_secrets_goal2")) )); } { // to_secrets2 not reachable Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // using symbolic in the intermediate path Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2/master.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_secrets_goal2 not reachable Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets_goal2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } // pattern search feature @@ -526,8 +526,8 @@ public class DirectoryBrowserSupportTest { List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); assertThat(entryNames, containsInAnyOrder( - p.getName() + "/intermediateFolder/public2.key", - p.getName() + "/public1.key" + "intermediateFolder/public2.key", + "public1.key" )); } { // all the outside folders / files are not included in the zip @@ -535,7 +535,7 @@ public class DirectoryBrowserSupportTest { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, contains("intermediateFolder/public2.key")); + assertThat(entryNames, contains("public2.key")); } } @@ -595,17 +595,7 @@ public class DirectoryBrowserSupportTest { { // without the patch the otherFolder and to_secrets[1,2,3] will appear in the results (once) Page page = wc.goTo(p.getUrl() + "ws/**/goal.txt", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, allOf( - // really not satisfying the query - not(containsString("public1.key")), - not(containsString("public2.key")), - // those following presences would have leak information that there is some file satisfying that pattern inside - not(containsString("to_secrets")), - not(containsString("to_secrets2")), - not(containsString("to_secrets3")) - )); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // without the patch the otherFolder and to_secrets[1,2,3] will appear in the results (3 times each) Page page = wc.goTo(p.getUrl() + "ws/**/public*.key", null); @@ -669,24 +659,24 @@ public class DirectoryBrowserSupportTest { assertThat(workspaceContent, allOf( containsString("public1.key"), containsString("intermediateFolder"), - containsString("to_secrets1j"), - containsString("to_secrets1s"), - containsString("to_secrets_goal1"), + not(containsString("to_secrets1j")), + not(containsString("to_secrets1s")), + not(containsString("to_secrets_goal1")), not(containsString("to_secrets2")), not(containsString("to_secrets_goal2")) )); } { // to_secrets1s not reachable Page page = wc.goTo(p.getUrl() + "ws/to_secrets1s/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_secrets1j not reachable Page page = wc.goTo(p.getUrl() + "ws/to_secrets1j/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_secrets_goal1 not reachable Page page = wc.goTo(p.getUrl() + "ws/to_secrets_goal1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // intermediateFolder must be reachable (regular case) Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/", null); @@ -695,30 +685,30 @@ public class DirectoryBrowserSupportTest { assertThat(workspaceContent, allOf( not(containsString("to_secrets1")), not(containsString("to_secrets_goal1")), - containsString("to_secrets2s"), - containsString("to_secrets2j"), - containsString("to_secrets_goal2") + not(containsString("to_secrets2s")), + not(containsString("to_secrets2j")), + not(containsString("to_secrets_goal2")) )); } { // to_secrets2s not reachable Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2s/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_secrets2j not reachable Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2j/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // using symbolic in the intermediate path Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2s/master.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // using symbolic in the intermediate path Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets2j/master.key", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_secrets_goal2 not reachable Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_secrets_goal2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_FORBIDDEN)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } // pattern search feature @@ -749,8 +739,8 @@ public class DirectoryBrowserSupportTest { List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); assertThat(entryNames, containsInAnyOrder( - p.getName() + "/intermediateFolder/public2.key", - p.getName() + "/public1.key" + "intermediateFolder/public2.key", + "public1.key" )); } { // all the outside folders / files are not included in the zip @@ -758,7 +748,7 @@ public class DirectoryBrowserSupportTest { assertThat(zipPage.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); List entryNames = getListOfEntriesInDownloadedZip((UnexpectedPage) zipPage); - assertThat(entryNames, contains("intermediateFolder/public2.key")); + assertThat(entryNames, contains("public2.key")); } } @@ -844,8 +834,8 @@ public class DirectoryBrowserSupportTest { } @Test - @Issue("SECURITY-904") - public void symlink_insideWorkspace_areStillAllowed() throws Exception { + @Issue({"SECURITY-904", "SECURITY-1452"}) + public void symlink_insideWorkspace_areNotAllowedAnymore() throws Exception { FreeStyleProject p = j.createFreeStyleProject(); // build once to have the workspace set up @@ -886,8 +876,8 @@ public class DirectoryBrowserSupportTest { String workspaceContent = page.getWebResponse().getContentAsString(); assertThat(workspaceContent, allOf( containsString("asset"), - containsString("to_internal1"), - containsString("to_internal_goal1"), + not(containsString("to_internal1")), + not(containsString("to_internal_goal1")), containsString("intermediateFolder"), not(containsString("to_internal2")), not(containsString("to_internal_goal2") @@ -895,27 +885,19 @@ public class DirectoryBrowserSupportTest { } { // to_internal1 reachable Page page = wc.goTo(p.getUrl() + "ws/to_internal1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString("goal.txt")); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_internal_goal1 reachable Page page = wc.goTo(p.getUrl() + "ws/to_internal_goal1/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString(publicContent)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_internal2 reachable Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_internal2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString("goal.txt")); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // to_internal_goal2 reachable Page page = wc.goTo(p.getUrl() + "ws/intermediateFolder/to_internal_goal2/", null); - assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_OK)); - String workspaceContent = page.getWebResponse().getContentAsString(); - assertThat(workspaceContent, containsString(publicContent)); + assertThat(page.getWebResponse().getStatusCode(), equalTo(HttpURLConnection.HTTP_NOT_FOUND)); } { // direct to goal Page page = wc.goTo(p.getUrl() + "ws/asset/goal.txt/", null); @@ -945,4 +927,158 @@ public class DirectoryBrowserSupportTest { File resourceFile = new File(resourceUrl.toURI()); return FileUtils.readFileToString(resourceFile, StandardCharsets.UTF_8); } + + public static final class SimulatedExternalArtifactManagerFactory extends ArtifactManagerFactory { + @Override + public ArtifactManager managerFor(Run build) { + return new SimulatedExternalArtifactManager(); + } + @TestExtension("externalURLDownload") + public static final class DescriptorImpl extends ArtifactManagerFactoryDescriptor {} + } + + private static final class SimulatedExternalArtifactManager extends ArtifactManager { + String hash; + + @Override + public void archive(FilePath workspace, Launcher launcher, BuildListener listener, Map artifacts) throws IOException, InterruptedException { + assertEquals(1, artifacts.size()); + Map.Entry entry = artifacts.entrySet().iterator().next(); + assertEquals("f", entry.getKey()); + try (InputStream is = workspace.child(entry.getValue()).read()) { + byte[] data = IOUtils.toByteArray(is); + ExtensionList.lookupSingleton(ContentAddressableStore.class).files.add(data); + hash = Util.getDigestOf(new ByteArrayInputStream(data)); + } + } + + @Override + public VirtualFile root() { + final VirtualFile file = new VirtualFile() { // the file inside the root + @Override + public String getName() { + return "f"; + } + @Override + public URI toURI() { + return URI.create("root:f"); + } + @Override + public VirtualFile getParent() { + return root(); + } + @Override + public boolean isDirectory() { + return false; + } + @Override + public boolean isFile() { + return true; + } + @Override + public boolean exists() { + return true; + } + @Override + public VirtualFile[] list() { + return new VirtualFile[0]; + } + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes) { + return Collections.emptySet(); + } + @Override + public VirtualFile child(String name) { + throw new UnsupportedOperationException(); + } + @Override + public long length() { + return 0; + } + @Override + public long lastModified() { + return 0; + } + @Override + public boolean canRead() { + return true; + } + @Override + public InputStream open() throws IOException { + throw new FileNotFoundException("expect to be opened via URL only"); + } + @Override + public URL toExternalURL() throws IOException { + return new URL(Jenkins.get().getRootUrl() + "files/" + hash); + } + }; + return new VirtualFile() { // the root + @Override + public String getName() { + return ""; + } + @Override + public URI toURI() { + return URI.create("root:"); + } + @Override + public VirtualFile getParent() { + return this; + } + @Override + public boolean isDirectory() { + return true; + } + @Override + public boolean isFile() { + return false; + } + @Override + public boolean exists() { + return true; + } + @Override + public VirtualFile[] list() { + return new VirtualFile[] {file}; + } + @Override + public Collection list(String includes, String excludes, boolean useDefaultExcludes) { + throw new UnsupportedOperationException(); + } + @Override + public VirtualFile child(String name) { + if (name.equals("f")) { + return file; + } else if (name.isEmpty()) { + return this; + } else { + throw new UnsupportedOperationException("trying to call child on " + name); + } + } + @Override + public long length() { + return 0; + } + @Override + public long lastModified() { + return 0; + } + @Override + public boolean canRead() { + return true; + } + @Override + public InputStream open() throws IOException { + throw new FileNotFoundException(); + } + }; + } + @Override + public void onLoad(Run build) {} + @Override + public boolean delete() { + return false; + } + } + } diff --git a/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java b/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java index 87d580f94b..b880dc0356 100644 --- a/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java +++ b/test/src/test/java/hudson/tasks/ArtifactArchiverTest.java @@ -238,7 +238,7 @@ public class ArtifactArchiverTest { assertFalse(kids[0].isDirectory()); assertFalse(kids[0].isFile()); assertFalse(kids[0].exists()); - j.createWebClient().assertFails(b.getUrl() + "artifact/hack", HttpURLConnection.HTTP_FORBIDDEN); + j.createWebClient().assertFails(b.getUrl() + "artifact/hack", HttpURLConnection.HTTP_NOT_FOUND); } static class CreateArtifact extends TestBuilder {