Cross file-system path handling

api/src/org/labkey/api/ApiModule.java

@@ -23,6 +23,7 @@
 import org.jetbrains.annotations.NotNull;
 import org.json.JSONObject;
 import org.labkey.api.action.ApiQueryResponse;
+import org.labkey.api.action.ApiResponseWriter;
 import org.labkey.api.action.ApiXmlWriter;
 import org.labkey.api.action.ConcurrencyLimiter;
 import org.labkey.api.action.SpringActionController;
@@ -414,6 +415,7 @@ public void registerServlets(ServletContext servletCtx)
     {
         return Set.of(
             Aggregate.TestCase.class,
+            ApiResponseWriter.TestCase.class,
             ApiXmlWriter.TestCase.class,
             ArrayListMap.TestCase.class,
             AssayResultsFileWriter.TestCase.class,

api/src/org/labkey/api/action/ApiResponseWriter.java

@@ -21,13 +21,16 @@
 import org.jetbrains.annotations.Nullable;
 import org.json.JSONArray;
 import org.json.JSONObject;
+import org.junit.Assert;
+import org.junit.Test;
 import org.labkey.api.query.BatchValidationException;
 import org.labkey.api.query.PropertyValidationError;
 import org.labkey.api.query.SimpleValidationError;
 import org.labkey.api.query.ValidationError;
 import org.labkey.api.query.ValidationException;
 import org.labkey.api.util.ExceptionUtil;
 import org.labkey.api.util.ExpectedException;
+import org.labkey.api.util.FileUtil;
 import org.labkey.api.util.JsonUtil;
 import org.labkey.api.util.Pair;
 import org.labkey.api.util.UnexpectedException;
@@ -40,6 +43,7 @@
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.io.StringWriter;
 import java.io.Writer;
 import java.util.List;
 import java.util.Stack;
@@ -232,6 +236,9 @@ public int getErrorResponseStatus(Throwable ex)
         if (ex instanceof HttpStatusException statusEx)
             return statusEx.getStatus();
 
+        if (ex instanceof FileUtil.InvalidPathReferenceException)
+            return HttpServletResponse.SC_NOT_FOUND;
+
         if (ExceptionUtil.isRequestTimeout(ex))
             return HttpServletResponse.SC_SERVICE_UNAVAILABLE;
 
@@ -611,4 +618,15 @@ public void setSerializeViaJacksonAnnotations(boolean serializeViaJacksonAnnotat
     {
         this.serializeViaJacksonAnnotations = serializeViaJacksonAnnotations;
     }
+
+    public static class TestCase extends Assert
+    {
+        @Test
+        public void testErrorResponseStatus() throws IOException
+        {
+            ApiResponseWriter writer = new ApiJsonWriter(new StringWriter());
+            assertEquals(HttpServletResponse.SC_NOT_FOUND, writer.getErrorResponseStatus(new FileUtil.InvalidPathReferenceException("../x", "Path to parent not allowed")));
+            assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, writer.getErrorResponseStatus(new IllegalArgumentException()));
+        }
+    }
 }

api/src/org/labkey/api/docker/DockerService.java

@@ -25,7 +25,6 @@
 import org.labkey.vfs.FileLike;
 
 import java.io.File;
-import java.io.FileFilter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -36,6 +35,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Predicate;
 
 // Moved from git, see history at https://github.com/LabKey/docker/commits/release18.1/api-src/org/labkey/api/docker/DockerService.java
 
@@ -463,7 +463,7 @@ public DockerContainer(
 
     void start(String containerId);
 
-    void stop(String containerId, Map<String, String> archivesFromContainer, boolean delete);
+    void stop(String containerId, Map<String, FileLike> archivesFromContainer, boolean delete);
 
     void stop(DockerContainer dc);
 
@@ -481,7 +481,7 @@ public DockerContainer(
 
     boolean isUseDockerVolumes();
 
-    default void executeR(DockerImage dockerImage, FileLike scriptFile, String localWorkingDir, String remoteWorkingDir, FileFilter inputScripts) throws IOException
+    default void executeR(DockerImage dockerImage, FileLike scriptFile, FileLike localWorkingDir, String remoteWorkingDir, Predicate<FileLike> inputScripts) throws IOException
     {
         throw new UnsupportedOperationException(NO_DOCKER);
     }

api/src/org/labkey/api/reports/report/r/RDockerScriptEngine.java

@@ -29,14 +29,15 @@
 import org.labkey.api.util.FileUtil;
 import org.labkey.api.util.UnexpectedException;
 import org.labkey.vfs.FileLike;
+import org.labkey.vfs.FileSystemLike;
 
 import javax.script.ScriptContext;
 import javax.script.ScriptException;
 import java.io.File;
-import java.io.FileFilter;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Collections;
+import java.util.function.Predicate;
 
 /**
  * Created by matthew on 7/14/17.
@@ -100,7 +101,9 @@ protected Object eval(FileLike scriptFile, ScriptContext context) throws ScriptE
         {
             try (CustomTiming t = MiniProfiler.custom("docker", "execute r in docker container"))
             {
-                _ds.executeR(_dockerImage, scriptFile, getRWorkingDir(context), _remoteWorkingDir, InputFiles());
+                // root the file system at the working dir so files copied back from the container can't escape it
+                FileLike workingDir = new FileSystemLike.Builder(getWorkingDir(context).toNioPathForWrite()).readwrite().root();
+                _ds.executeR(_dockerImage, scriptFile, workingDir, _remoteWorkingDir, InputFiles());
                 appendConsoleOutput(context, output);
             }
             catch (Exception e)
@@ -122,7 +125,7 @@ protected Object eval(FileLike scriptFile, ScriptContext context) throws ScriptE
     /**
      * Filter for the input script files in the report temp folder to copy to the Docker container which will run R
      */
-    private static FileFilter InputFiles()
+    private static Predicate<FileLike> InputFiles()
     {
         return pathname ->
                 pathname.isFile() &&

api/src/org/labkey/api/util/FileUtil.java

@@ -885,12 +885,15 @@ public static File appendPath(File dir, org.labkey.api.util.Path originalPath)
 
 
     /** Only returns a child path */
-    public static FileLike appendPath(FileLike dir, org.labkey.api.util.Path path)
+    public static FileLike appendPath(FileLike dir, org.labkey.api.util.Path originalPath)
     {
-        path = path.normalize();
-        if (!path.isEmpty() && "..".equals(path.get(0)))
-            throw new InvalidPathReferenceException(path.toString(), "Path to parent not allowed");
-        return dir.resolveFile(path);
+        // normalize() returns null rather than a path with a leading ".."
+        org.labkey.api.util.Path path = originalPath.normalize();
+        assert path == null || path.isEmpty() || !path.get(0).equals("..");
+        if (path == null || (!path.isEmpty() && "..".equals(path.get(0))))
+            throw new InvalidPathReferenceException(originalPath.toString(), "Path to parent not allowed");
+        // resolveFile() resolves an absolute path from the file system root, not from dir
+        return dir.resolveFile(path.relative());
     }
 
 
@@ -2480,6 +2483,18 @@ public void testNoAcceptableExtensions()
             assertNull("Numeric extension should be allowed", checkExtension("test.1", mockProps));
         }
 
+        @Test
+        public void testAppendPathFileLike()
+        {
+            FileLike sub = new FileSystemLike.Builder(ROOT).root().resolveChild("sub");
+            FileLike x = sub.resolveChild("x");
+            assertEquals(x, appendPath(sub, org.labkey.api.util.Path.parse("a/../x")));
+            assertEquals(x, appendPath(sub, org.labkey.api.util.Path.parse("/x")));
+            assertEquals(sub, appendPath(sub, org.labkey.api.util.Path.parse("/")));
+            assertThrows(InvalidPathReferenceException.class, () -> appendPath(sub, org.labkey.api.util.Path.parse("../x")));
+            assertThrows(InvalidPathReferenceException.class, () -> appendPath(sub, org.labkey.api.util.Path.parse("a/../../x")));
+        }
+
         @Test
         public void testGetAppendedFileName()
         {

api/src/org/labkey/api/util/Path.java

@@ -395,6 +395,11 @@ public Path getParent()
         return _parent.get();
     }
 
+
+    /**
+     * Create a path that does not contain parts "." or "..".
+     * NOTE: returns null if the normalized path would need to start with "../"
+     */
     public Path normalize()
     {
         for (int i = 0; i < _length; i++)

api/src/org/labkey/vfs/AbstractFileLike.java

@@ -18,6 +18,7 @@
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import org.labkey.api.util.FileUtil;
 import org.labkey.api.util.Path;
 import org.labkey.api.view.UnauthorizedException;
 
@@ -117,7 +118,7 @@ static Path resolvePath(Path base, Path path)
         var resolved = base.resolve(path);
         resolved = resolved.normalize();
         if (null == resolved)
-            throw new IllegalArgumentException("Illegal path " + path + " in " + base);
+            throw new FileUtil.InvalidPathReferenceException(path.toString(), "Illegal path in " + base);
         assert resolved.isAbsolute();
         return resolved;
     }

api/src/org/labkey/vfs/FileLike.java

@@ -37,7 +37,6 @@
 import java.io.OutputStream;
 import java.net.URI;
 import java.nio.file.Files;
-import java.nio.file.InvalidPathException;
 import java.util.List;
 import java.util.function.Predicate;
 
@@ -116,10 +115,10 @@ default FileLike resolveChild(Path.Part name)
     default FileLike resolveChild(String name)
     {
         if (".".equals(name) || "..".equals(name))
-            throw new InvalidPathException(name, "Cannot resolve child");
+            throw new FileUtil.InvalidPathReferenceException(name, "Cannot resolve child");
         Path path = Path.parse(name);
         if (1 != path.size())
-            throw new InvalidPathException(name, "Cannot resolve child");
+            throw new FileUtil.InvalidPathReferenceException(name, "Cannot resolve child");
         return resolveFile(path);
     }
 

api/src/org/labkey/vfs/FileSystemLike.java

@@ -293,11 +293,7 @@ static FileLike wrapFile(java.nio.file.Path p)
     /** Helper for partially converted code. root must exist. */
     static FileLike wrapFile(File root, File f) throws IOException
     {
-        if (!root.isDirectory())
-            throw new FileNotFoundException(root.getPath());
-        FileSystemLike fs = new Builder(root.toURI()).build();
-        String rel = FileUtil.relativize(root, f, true);
-        return fs.getRoot().resolveFile(Path.parse(rel));
+        return wrapFile(FileUtil.getAbsoluteCaseSensitiveFile(root).toPath(), FileUtil.getAbsoluteCaseSensitiveFile(f).toPath());
     }
 
     /** Helper for partially converted code. root must exist. */
@@ -311,7 +307,7 @@ static FileLike wrapFile(java.nio.file.Path root, java.nio.file.Path f) throws I
         {
             throw new IOException("File '" + f.toUri().getPath() + "' is outside the allowed root '" + root.toUri().getPath() + "'");
         }
-        return fs.resolveFile(new Path(relative.getPath()));
+        return fs.resolveFile(Path.parse(relative.getPath()));
     }
 
     /** Helper for partially converted code. May throw if the FileLike does not wrap a local file system. */
@@ -408,7 +404,7 @@ static FileLike getVerifiedFileLike(Container container, String filePath)
         }
 
         // if root = /a/b/c/ and file = /a/b/c/d/e/f.xlsx - among other things, this essentially checks if '/a/b/c/d/e/f.xlsx' starts with '/a/b/c/'
-        return allowedRoot.resolveFile(new Path(relativeURI.getPath()));
+        return allowedRoot.resolveFile(Path.parse(relativeURI.getPath()));
     }
 }
 

api/src/org/labkey/vfs/FileSystemLocal.java

@@ -91,13 +91,32 @@ public FileLike getRoot()
     public FileLike resolveFile(Path path)
     {
         LOG.debug("Resolving {} against {}", path, nioRoot);
-        path = path.absolute().normalize();
-        if (null == path)
-            throw new IllegalArgumentException("Path could not be resolved");
+        var normalized = path.absolute().normalize();
+        if (null == normalized)
+            throw new FileUtil.InvalidPathReferenceException(path.toString(), "Path could not be resolved");
         var nioPath = nioRoot;
-        for (var s : path)
-            nioPath = nioPath.resolve(s);
-        return createFileLike(path, nioPath.toFile());
+        for (var s : normalized)
+            nioPath = nioPath.resolve(toNioName(s));
+        if (!nioPath.normalize().startsWith(nioRoot.normalize()))
+            throw new FileUtil.InvalidPathReferenceException(path.toString(), "Path is outside the file system root");
+        return createFileLike(normalized, nioPath.toFile());
+    }
+
+    // Each Path part must map to exactly one nio name; reject a part nio would split or treat as a root (e.g. "..\x" or "C:x" on Windows)
+    private java.nio.file.Path toNioName(String part)
+    {
+        java.nio.file.Path name;
+        try
+        {
+            name = nioRoot.getFileSystem().getPath(part);
+        }
+        catch (InvalidPathException x)
+        {
+            throw new FileUtil.InvalidPathReferenceException(part, x.getReason());
+        }
+        if (name.getRoot() != null || name.getNameCount() != 1 || ".".equals(part) || "..".equals(part))
+            throw new FileUtil.InvalidPathReferenceException(part, "Not a valid file name");
+        return name;
     }
 
 

api/src/org/labkey/vfs/vfsTestCase.jsp

@@ -17,6 +17,9 @@
 <%@ page import="org.junit.Before" %>
 <%@ page import="org.junit.Test" %>
 <%@ page import="java.io.File" %>
+<%@ page import="java.io.IOException" %>
+<%@ page import="java.io.FileNotFoundException" %>
+<%@ page import="java.nio.file.Files" %>
 <%@ page import="java.net.URI" %>
 <%@ page import="org.labkey.api.util.FileUtil" %>
 <%@ page import="org.labkey.vfs.FileSystemLike" %>
@@ -69,14 +72,20 @@
 
             FileLike tempDir = new FileSystemLike.Builder(tempPath.toFile()).root();
             assertNotNull(tempDir);
-            FileLike t1 = tempDir.resolveFile(Path.parse(tempPath + "\\b\\t1.txt"));
+            FileLike t1 = tempDir.resolveFile(Path.parse("b/t1.txt"));
             assertNotNull(t1);
-            FileLike td1 = tempDir.resolveFile(Path.parse(tempPath + "\\b\\"));
+            FileLike td1 = tempDir.resolveFile(Path.parse("b/"));
             assertNotNull(td1);
             FileLike tc1 = td1.resolveChild("t1.txt");
             assertNotNull(tc1);
             assertEquals(t1, tc1);
 
+            for (String name : new String[] {tempPath + "\\b\\t1.txt", "..\\x", "a\\..\\..\\x", "a\\b", "C:\\x", "C:x", "\\x", "\\\\host\\share\\x", "a<b.txt", "a|b"})
+            {
+                assertThrows(name, FileUtil.InvalidPathReferenceException.class, () -> tempDir.resolveFile(new Path(name)));
+                assertThrows(name, FileUtil.InvalidPathReferenceException.class, () -> tempDir.resolveChild(name));
+            }
+
             FileLike root = new FileSystemLike.Builder(new File("C:\\")).root();
             assertNotNull(root);
             FileLike r1 = root.resolveFile(Path.parse("r1.txt"));
@@ -203,9 +212,65 @@
     }
 
 
-    void resolve(FileSystemLike fs)
+    @Test
+    public void testWrapFile() throws Exception
     {
+        File root = tempPath.toFile();
+        java.nio.file.Path nested = Files.createFile(Files.createDirectory(tempPath.resolve("sub")).resolve("f.txt"));
+
+        FileLike fromFile = FileSystemLike.wrapFile(root, nested.toFile());
+        assertEquals(2, fromFile.getPath().size());
+        assertEquals("sub", fromFile.getPath().get(0));
+        assertEquals("f.txt", fromFile.getName());
+        assertTrue(fromFile.isFile());
+
+        FileLike fromNio = FileSystemLike.wrapFile(tempPath, nested);
+        assertEquals(fromFile, fromNio);
+        assertTrue(fromNio.isFile());
+
+        assertThrows(IOException.class, () -> FileSystemLike.wrapFile(root, new File(root.getParentFile(), "x.txt")));
+        assertThrows(IOException.class, () -> FileSystemLike.wrapFile(root, new File(new File(root.getParentFile(), root.getName() + "foo"), "x.txt")));
+        assertThrows(IOException.class, () -> FileSystemLike.wrapFile(root, new File(root, "sub/../../x.txt")));
+        assertEquals(fromFile, FileSystemLike.wrapFile(root, new File(root, "sub/../sub/f.txt")));
+
+        FileLike self = FileSystemLike.wrapFile(root, root);
+        assertTrue(self.getPath().isEmpty());
+        assertTrue(self.isDirectory());
+
+        // URI round trip must not decode the name twice
+        java.nio.file.Path encoded = Files.createFile(tempPath.resolve("a b%20c.txt"));
+        FileLike e = FileSystemLike.wrapFile(root, encoded.toFile());
+        assertEquals(1, e.getPath().size());
+        assertEquals("a b%20c.txt", e.getName());
+        assertTrue(e.isFile());
+
+        assertThrows(FileNotFoundException.class, () -> FileSystemLike.wrapFile(nested.toFile(), nested.toFile()));
+
+        if (!System.getProperty("os.name").toLowerCase().startsWith("windows"))
+        {
+            // '\' is an ordinary file name character here
+            java.nio.file.Path backslash = Files.createFile(tempPath.resolve("a\\b.txt"));
+            FileLike f = FileSystemLike.wrapFile(root, backslash.toFile());
+            assertEquals(1, f.getPath().size());
+            assertEquals("a\\b.txt", f.getName());
+            assertTrue(f.isFile());
+        }
+    }
+
 
+    void resolve(FileSystemLike fs)
+    {
+        FileLike root = fs.getRoot();
+        FileLike a = root.resolveChild("a");
+        assertEquals(a, root.resolveFile(Path.parse("b/../a")));
+        assertEquals("b.txt", a.resolveFile(new Path("b.txt")).getName());
+
+        // a Path part is a single file name, never a path
+        for (String name : new String[] {"../x", "a/../../x", "a/b", "/x"})
+            assertThrows(name, FileUtil.InvalidPathReferenceException.class, () -> a.resolveFile(new Path(name)));
+        for (String name : new String[] {"..", "../x", "a/b"})
+            assertThrows(name, FileUtil.InvalidPathReferenceException.class, () -> a.resolveChild(name));
+        assertThrows(FileUtil.InvalidPathReferenceException.class, () -> a.resolveFile(Path.parse("../../x")));
     }
 
     void list(FileSystemLike fs)