From 407fe75f108cc939bc235b0b1426911018890d00 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 25 Sep 2025 16:55:33 -0700 Subject: [PATCH 1/6] Path related warnings --- .../org/labkey/api/assay/actions/AssayRunUploadForm.java | 3 ++- api/src/org/labkey/api/exp/AbstractFileXarSource.java | 3 +++ api/src/org/labkey/api/pipeline/AnalyzeForm.java | 3 ++- api/src/org/labkey/api/pipeline/LocalDirectory.java | 4 ++-- api/src/org/labkey/api/query/AbstractQueryImportAction.java | 2 +- api/src/org/labkey/api/util/FileType.java | 6 +++--- .../experiment/controllers/exp/ExperimentController.java | 4 ++-- .../experiment/pipeline/ExperimentPipelineProvider.java | 4 ++-- .../org/labkey/filecontent/FileSystemAttachmentParent.java | 2 +- .../src/org/labkey/pipeline/api/PipelineServiceImpl.java | 2 +- study/src/org/labkey/study/assay/StudyPublishManager.java | 2 +- study/src/org/labkey/study/importer/StudyImportContext.java | 2 +- 12 files changed, 21 insertions(+), 16 deletions(-) diff --git a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java index 583dc3de7d0..dfab0aa4eda 100644 --- a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java +++ b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java @@ -59,6 +59,7 @@ import org.labkey.api.study.StudyService; import org.labkey.api.study.publish.StudyPublishService; import org.labkey.api.study.assay.ParticipantVisitResolverType; +import org.labkey.api.util.FileUtil; import org.labkey.api.util.GUID; import org.labkey.api.view.ActionURL; import org.labkey.api.view.NotFoundException; @@ -360,7 +361,7 @@ public Map getAdditionalPostedFiles(List { try { - Files.move(inputFile, dirData.resolve(inputFile.getFileName().toString())); + Files.move(inputFile, FileUtil.appendName(dirData, inputFile.getFileName().toString())); } catch (IOException e) { diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 35888fa0a36..d4a2cc2c894 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -958,7 +958,7 @@ public UploadLog saveUploadData(User user, Dataset dsd, FileStream tsv, String f String extra = id++ == 0 ? "" : String.valueOf(id); String fileName = dsd.getStudy().getLabel() + "-" + dsd.getLabel() + "-" + dateString + extra + "." + extension; fileName = fileName.replace('\\', '_').replace('/', '_').replace(':', '_'); - file = dir.resolve(fileName); + file = FileUtil.appendName(dir, fileName); } while (Files.exists(file)); diff --git a/study/src/org/labkey/study/importer/StudyImportContext.java b/study/src/org/labkey/study/importer/StudyImportContext.java index 2d81c65b41e..1adf4a587f5 100644 --- a/study/src/org/labkey/study/importer/StudyImportContext.java +++ b/study/src/org/labkey/study/importer/StudyImportContext.java @@ -131,7 +131,7 @@ private Path getStudyFile(VirtualFile root, VirtualFile dir, String name) throws { Path rootFile = FileUtil.stringToPath(getContainer(), root.getLocation()); Path dirFile = FileUtil.stringToPath(getContainer(), dir.getLocation()); - Path file = dirFile.resolve(name); + Path file = FileUtil.appendName(dirFile, name); String source = "study.xml"; if (!Files.exists(file)) From 57f516054dad89ecc00855f349706db305aee40b Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 25 Sep 2025 21:19:26 -0700 Subject: [PATCH 2/6] Path related warnings --- api/src/org/labkey/api/query/AbstractQueryImportAction.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index e7976398cd9..bf164a3b67e 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -483,6 +483,9 @@ else if (StringUtils.isNotBlank(path)) if (!root.isUnderRoot(f)) f = root.resolvePath(path); + if (f == null || !f.isFile() || !f.toPath().normalize().startsWith(root.getRootFileLike().toNioPathForRead().normalize())) + throw new IllegalArgumentException("Invalid path: " + path); + if (root.isUnderRoot(f) && NetworkDrive.exists(f) && root.hasPermission(getContainer(), getUser(), ReadPermission.class)) { hasPostData = true; From a5476a1b87cad3e3595debe70c8cd6664af871a2 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 26 Sep 2025 10:10:12 -0700 Subject: [PATCH 3/6] Fix import using path --- .../api/attachments/FileAttachmentFile.java | 4 ++-- .../api/query/AbstractQueryImportAction.java | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/api/src/org/labkey/api/attachments/FileAttachmentFile.java b/api/src/org/labkey/api/attachments/FileAttachmentFile.java index 3e8eec51918..8dab0a1e109 100644 --- a/api/src/org/labkey/api/attachments/FileAttachmentFile.java +++ b/api/src/org/labkey/api/attachments/FileAttachmentFile.java @@ -47,12 +47,12 @@ public FileAttachmentFile(File file) public FileAttachmentFile(FileLike file) { - this(file.toNioPathForRead().toFile(), null); + this(file == null ? null : file.toNioPathForRead().toFile(), null); } public FileAttachmentFile(FileLike file, @Nullable String originalName) { - this(file.toNioPathForRead().toFile(), originalName); + this(file == null ? null : file.toNioPathForRead().toFile(), originalName); } public FileAttachmentFile(File file, @Nullable String originalName) diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index bf164a3b67e..d4a7837db8e 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -478,14 +478,23 @@ else if (StringUtils.isNotBlank(path)) PipeRoot root = PipelineService.get().findPipelineRoot(getContainer()); if (root != null) { - // Attempt absolute path first, then relative path from pipeline root - File f = new File(path); - if (!root.isUnderRoot(f)) - f = root.resolvePath(path); + Path relativePath = Path.parse(path); + java.nio.file.Path parsedPath = java.nio.file.Paths.get(path); + if (parsedPath.isAbsolute()) + { + String relative = root.relativePath(parsedPath); + if (relative == null) + throw new IllegalArgumentException("Invalid path: " + path); + relativePath = Path.parse(relative); + } - if (f == null || !f.isFile() || !f.toPath().normalize().startsWith(root.getRootFileLike().toNioPathForRead().normalize())) + // Attempt absolute path first, then relative path from pipeline root + FileLike fileLike = FileUtil.appendPath(root.getRootFileLike(), relativePath); + if (fileLike == null) throw new IllegalArgumentException("Invalid path: " + path); + File f = fileLike.toNioPathForRead().toFile(); + if (root.isUnderRoot(f) && NetworkDrive.exists(f) && root.hasPermission(getContainer(), getUser(), ReadPermission.class)) { hasPostData = true; From 8ff2a67cdbb498fcef2f510e986e91d15e2c7bac Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 26 Sep 2025 16:22:06 -0700 Subject: [PATCH 4/6] File path warning --- api/src/org/labkey/api/files/FileContentService.java | 2 +- .../org/labkey/api/query/AbstractQueryImportAction.java | 1 - .../org/labkey/filecontent/FileContentServiceImpl.java | 8 ++++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/files/FileContentService.java b/api/src/org/labkey/api/files/FileContentService.java index c78a0d74c59..9791ba68902 100644 --- a/api/src/org/labkey/api/files/FileContentService.java +++ b/api/src/org/labkey/api/files/FileContentService.java @@ -218,7 +218,7 @@ enum ContentType { * relative to the first parent container with an override */ File getDefaultRoot(Container c, boolean createDir); - Path getDefaultRootPath(Container c, boolean createDir); + Path getDefaultRootPath(@NotNull Container c, boolean createDir); class DefaultRootInfo { diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index d4a7837db8e..7658af9c4ea 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -488,7 +488,6 @@ else if (StringUtils.isNotBlank(path)) relativePath = Path.parse(relative); } - // Attempt absolute path first, then relative path from pipeline root FileLike fileLike = FileUtil.appendPath(root.getRootFileLike(), relativePath); if (fileLike == null) throw new IllegalArgumentException("Invalid path: " + path); diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index 436f9ff9940..da9fe34706a 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -310,7 +310,7 @@ public File getDefaultRoot(Container c, boolean createDir) } @Override - public java.nio.file.Path getDefaultRootPath(Container c, boolean createDir) + public java.nio.file.Path getDefaultRootPath(@NotNull Container c, boolean createDir) { Container firstOverride = getFirstAncestorWithOverride(c); @@ -325,7 +325,7 @@ public java.nio.file.Path getDefaultRootPath(Container c, boolean createDir) parentRoot = getFileRootPath(firstOverride); } - if (parentRoot != null && c != null && firstOverride != null) + if (parentRoot != null && firstOverride != null) { java.nio.file.Path fileRootPath; if (FileUtil.hasCloudScheme(parentRoot)) @@ -336,7 +336,7 @@ public java.nio.file.Path getDefaultRootPath(Container c, boolean createDir) else { // For local, the path may be several directories deep (since it matches the LK folder path), so we should create the directories for that path - fileRootPath = new File(parentRoot.toFile(), getRelativePath(c, firstOverride)).toPath(); + fileRootPath = FileUtil.appendPath(parentRoot.toFile(), Path.parse(getRelativePath(c, firstOverride))).toPath(); try { @@ -395,7 +395,7 @@ public FileRoot getDefaultFileRoot(Container c) return null; } - private String getRelativePath(Container c, Container ancestor) + private @NotNull String getRelativePath(Container c, Container ancestor) { return c.getPath().replaceAll("^" + Pattern.quote(ancestor.getPath()), ""); } From c125e9ac6f6dd70d993349ea816aa63f0dab0a41 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 26 Sep 2025 17:03:53 -0700 Subject: [PATCH 5/6] don't allow non dav path for import action. --- .../api/attachments/FileAttachmentFile.java | 13 ++++---- .../api/query/AbstractQueryImportAction.java | 31 ------------------- 2 files changed, 7 insertions(+), 37 deletions(-) diff --git a/api/src/org/labkey/api/attachments/FileAttachmentFile.java b/api/src/org/labkey/api/attachments/FileAttachmentFile.java index 8dab0a1e109..7021d847e96 100644 --- a/api/src/org/labkey/api/attachments/FileAttachmentFile.java +++ b/api/src/org/labkey/api/attachments/FileAttachmentFile.java @@ -17,6 +17,7 @@ package org.labkey.api.attachments; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.logging.LogHelper; @@ -40,22 +41,22 @@ public class FileAttachmentFile implements AttachmentFile private InputStream _in; - public FileAttachmentFile(File file) + public FileAttachmentFile(@NotNull File file) { this(file, null); } - public FileAttachmentFile(FileLike file) + public FileAttachmentFile(@NotNull FileLike file) { - this(file == null ? null : file.toNioPathForRead().toFile(), null); + this(file.toNioPathForRead().toFile(), null); } - public FileAttachmentFile(FileLike file, @Nullable String originalName) + public FileAttachmentFile(@NotNull FileLike file, @Nullable String originalName) { - this(file == null ? null : file.toNioPathForRead().toFile(), originalName); + this(file.toNioPathForRead().toFile(), originalName); } - public FileAttachmentFile(File file, @Nullable String originalName) + public FileAttachmentFile(@NotNull File file, @Nullable String originalName) { _file = file; _filename = null != originalName ? originalName : file.getName(); diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index 7658af9c4ea..960ce45f70d 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -472,37 +472,6 @@ else if (StringUtils.isNotBlank(path)) file = resource.getFileStream(user); originalName = resource.getName(); } - else - { - // Resolve file under pipeline root - PipeRoot root = PipelineService.get().findPipelineRoot(getContainer()); - if (root != null) - { - Path relativePath = Path.parse(path); - java.nio.file.Path parsedPath = java.nio.file.Paths.get(path); - if (parsedPath.isAbsolute()) - { - String relative = root.relativePath(parsedPath); - if (relative == null) - throw new IllegalArgumentException("Invalid path: " + path); - relativePath = Path.parse(relative); - } - - FileLike fileLike = FileUtil.appendPath(root.getRootFileLike(), relativePath); - if (fileLike == null) - throw new IllegalArgumentException("Invalid path: " + path); - - File f = fileLike.toNioPathForRead().toFile(); - - if (root.isUnderRoot(f) && NetworkDrive.exists(f) && root.hasPermission(getContainer(), getUser(), ReadPermission.class)) - { - hasPostData = true; - loader = DataLoader.get().createLoader(f, null, _hasColumnHeaders, null, null); - file = new FileAttachmentFile(dataFile, f.getName()); - originalName = f.getName(); - } - } - } if (!hasPostData) { From 7e9243921a47c4b1ff4dd86384c2822ccc98ec8d Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 26 Sep 2025 22:11:17 -0700 Subject: [PATCH 6/6] allow checking against pipeline root --- api/src/org/labkey/api/exp/AbstractFileXarSource.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/exp/AbstractFileXarSource.java b/api/src/org/labkey/api/exp/AbstractFileXarSource.java index d70142fa392..1b57d449e29 100644 --- a/api/src/org/labkey/api/exp/AbstractFileXarSource.java +++ b/api/src/org/labkey/api/exp/AbstractFileXarSource.java @@ -20,7 +20,9 @@ import org.fhcrc.cpas.exp.xml.ExperimentArchiveDocument; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; +import org.labkey.api.pipeline.PipeRoot; import org.labkey.api.pipeline.PipelineJob; +import org.labkey.api.pipeline.PipelineService; import org.labkey.api.security.User; import org.labkey.api.util.FileUtil; import org.labkey.api.util.NetworkDrive; @@ -115,7 +117,13 @@ public String canonicalizeDataFileURL(String dataFileURL) { Path path = xarDirectory.resolve(dataFileURL); if (!path.normalize().startsWith(xarDirectory.normalize())) - throw new InvalidPathException(dataFileURL, "Path to parent not allowed"); + { + // check alternative - relative to container pipeline root + PipeRoot root = PipelineService.get().findPipelineRoot(getXarContext().getContainer()); + boolean valid = root != null && root.isUnderRoot(path); + if (!valid) + throw new InvalidPathException(dataFileURL, "Path to parent not allowed"); + } String result = _dataFileURLs.get(FileUtil.getAbsolutePath(getXarContext().getContainer(), path)); if (result != null) {