Static files: Fix tree to only provide items from expected folders (closes #20962) (#21001)

* Applies checks for root folders to static file tree service.

* Add integration tests.

* Fix ancestor test.

* Amends from code review.

* Integration test compatibility suppressions.

* Reverted breaking change in test base class.

src/Umbraco.Cms.Api.Management/Controllers/StaticFile/Tree/StaticFileTreeControllerBase.cs

@@ -34,14 +34,14 @@ public class StaticFileTreeControllerBase : FileSystemTreeControllerBase
 
     protected override IFileSystem FileSystem { get; }
 
-    protected string[] GetDirectories(string path) =>
+    protected override string[] GetDirectories(string path) =>
         IsTreeRootPath(path)
             ? _allowedRootFolders
             : IsAllowedPath(path)
                 ? _fileSystemTreeService.GetDirectories(path)
                 : Array.Empty<string>();
 
-    protected string[] GetFiles(string path)
+    protected override string[] GetFiles(string path)
         => IsTreeRootPath(path) || IsAllowedPath(path) == false
             ? Array.Empty<string>()
             : _fileSystemTreeService.GetFiles(path);

src/Umbraco.Cms.Api.Management/Services/FileSystem/FileSystemTreeServiceBase.cs

@@ -1,4 +1,4 @@
-using Umbraco.Cms.Api.Management.Extensions;
+using Umbraco.Cms.Api.Management.Extensions;
 using Umbraco.Cms.Api.Management.ViewModels.FileSystem;
 using Umbraco.Cms.Api.Management.ViewModels.Tree;
 using Umbraco.Cms.Core.IO;
@@ -68,12 +68,12 @@ public abstract class FileSystemTreeServiceBase : IFileSystemTreeService
             .ToArray();
     }
 
-    public string[] GetDirectories(string path) => FileSystem
+    public virtual string[] GetDirectories(string path) => FileSystem
         .GetDirectories(path)
         .OrderBy(directory => directory)
         .ToArray();
 
-    public string[] GetFiles(string path) => FileSystem
+    public virtual string[] GetFiles(string path) => FileSystem
         .GetFiles(path)
         .Where(FilterFile)
         .OrderBy(file => file)

src/Umbraco.Cms.Api.Management/Services/FileSystem/PhysicalFileSystemTreeService.cs

@@ -4,10 +4,31 @@ namespace Umbraco.Cms.Api.Management.Services.FileSystem;
 
 public class PhysicalFileSystemTreeService : FileSystemTreeServiceBase, IPhysicalFileSystemTreeService
 {
+    private static readonly string[] _allowedRootFolders = { $"{Path.DirectorySeparatorChar}App_Plugins", $"{Path.DirectorySeparatorChar}wwwroot" };
+
     private readonly IFileSystem _physicalFileSystem;
 
     protected override IFileSystem FileSystem => _physicalFileSystem;
 
     public PhysicalFileSystemTreeService(IPhysicalFileSystem physicalFileSystem) =>
         _physicalFileSystem = physicalFileSystem;
+
+    /// <inheritdoc/>
+    public override string[] GetDirectories(string path) =>
+        IsTreeRootPath(path)
+            ? _allowedRootFolders
+            : IsAllowedPath(path)
+                ? base.GetDirectories(path)
+                : Array.Empty<string>();
+
+    /// <inheritdoc/>
+    public override string[] GetFiles(string path)
+        => IsTreeRootPath(path) || IsAllowedPath(path) is false
+            ? []
+            : base.GetFiles(path);
+
+    private static bool IsTreeRootPath(string path) => path == Path.DirectorySeparatorChar.ToString();
+
+    private static bool IsAllowedPath(string path) => _allowedRootFolders.Contains(path) || _allowedRootFolders.Any(folder => path.StartsWith($"{folder}{Path.DirectorySeparatorChar}"));
+
 }

tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml

@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
 <Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
   <Suppression>
@@ -15,6 +15,69 @@
     <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
     <IsBaselineSuppression>true</IsBaselineSuppression>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.PartialViewTreeServiceTests.Can_Get_Ancestors_From_StyleSheet_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.PartialViewTreeServiceTests.Can_Get_PathViewModels_From_StyleSheet_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.PartialViewTreeServiceTests.Can_Get_Siblings_From_PartialView_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.ScriptTreeServiceTests.Can_Get_Ancestors_From_StyleSheet_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.ScriptTreeServiceTests.Can_Get_PathViewModels_From_StyleSheet_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.ScriptTreeServiceTests.Can_Get_Siblings_From_Script_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.StyleSheetTreeServiceTests.Can_Get_Ancestors_From_StyleSheet_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.StyleSheetTreeServiceTests.Can_Get_PathViewModels_From_StyleSheet_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees.StyleSheetTreeServiceTests.Can_Get_Siblings_From_StyleSheet_Tree_Service</Target>
+    <Left>lib/net10.0/Umbraco.Tests.Integration.dll</Left>
+    <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
+    <IsBaselineSuppression>true</IsBaselineSuppression>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0002</DiagnosticId>
     <Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Examine.Lucene.UmbracoExamine.IndexInitializer.#ctor(Umbraco.Cms.Core.Strings.IShortStringHelper,Umbraco.Cms.Core.PropertyEditors.PropertyEditorCollection,Umbraco.Cms.Core.PropertyEditors.MediaUrlGeneratorCollection,Umbraco.Cms.Core.Scoping.IScopeProvider,Microsoft.Extensions.Logging.ILoggerFactory,Microsoft.Extensions.Options.IOptions{Umbraco.Cms.Core.Configuration.Models.ContentSettings},Umbraco.Cms.Core.Services.ILocalizationService,Umbraco.Cms.Core.Services.IContentTypeService,Umbraco.Cms.Core.Services.IDocumentUrlService,Umbraco.Cms.Core.Services.ILanguageService)</Target>
@@ -29,4 +92,4 @@
     <Right>lib/net10.0/Umbraco.Tests.Integration.dll</Right>
     <IsBaselineSuppression>true</IsBaselineSuppression>
   </Suppression>
-</Suppressions>
+</Suppressions>

tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/FileSystemTreeServiceTestsBase.cs

@@ -36,21 +36,23 @@ public abstract class FileSystemTreeServiceTestsBase : UmbracoIntegrationTest
             GetStylesheetsFileSystem(),
             GetScriptsFileSystem(),
             null);
+
+        CreateFiles();
+    }
+
+    protected virtual void CreateFiles()
+    {
         for (int i = 0; i < 10; i++)
         {
-            using var stream = CreateStream(Path.Join("tests"));
+            using var stream = CreateStream();
             TestFileSystem.AddFile($"file{i}{FileExtension}", stream);
         }
     }
 
     protected static Stream CreateStream(string contents = null)
     {
-        if (string.IsNullOrEmpty(contents))
+        const string DefaultFileContent = "/* test */";
-        {
+        var bytes = Encoding.UTF8.GetBytes(contents ?? DefaultFileContent);
-            contents = "/* test */";
-        }
-
-        var bytes = Encoding.UTF8.GetBytes(contents);
         return new MemoryStream(bytes);
     }
 

tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/PartialViewTreeServiceTests.cs

@@ -15,7 +15,7 @@ public class PartialViewTreeServiceTests : FileSystemTreeServiceTestsBase
     protected override IFileSystem? GetPartialViewsFileSystem() => TestFileSystem;
 
     [Test]
-    public void Can_Get_Siblings_From_PartialView_Tree_Service()
+    public void Can_Get_Siblings()
     {
         var service = new PartialViewTreeService(FileSystems);
 
@@ -31,20 +31,20 @@ public class PartialViewTreeServiceTests : FileSystemTreeServiceTestsBase
     }
 
     [Test]
-    public void Can_Get_Ancestors_From_StyleSheet_Tree_Service()
+    public void Can_Get_Ancestors()
     {
         var service = new PartialViewTreeService(FileSystems);
 
         var path = Path.Join("tests", $"file5{FileExtension}");
-        FileSystemTreeItemPresentationModel[] treeModel = service.GetAncestorModels(path, true);
+        FileSystemTreeItemPresentationModel[] treeModels = service.GetAncestorModels(path, true);
 
-        Assert.IsNotEmpty(treeModel);
+        Assert.IsNotEmpty(treeModels);
-        Assert.AreEqual(treeModel.Length, 2);
+        Assert.AreEqual(treeModels.Length, 2);
-        Assert.AreEqual(treeModel[0].Name, "tests");
+        Assert.AreEqual(treeModels[0].Name, "tests");
     }
 
     [Test]
-    public void Can_Get_PathViewModels_From_StyleSheet_Tree_Service()
+    public void Can_Get_PathViewModels()
     {
         var service = new PartialViewTreeService(FileSystems);
 
@@ -60,7 +60,7 @@ public class PartialViewTreeServiceTests : FileSystemTreeServiceTestsBase
         var service = new PartialViewTreeService(FileSystems);
         for (int i = 0; i < 2; i++)
         {
-            using var stream = CreateStream(Path.Join("tests"));
+            using var stream = CreateStream();
             TestFileSystem.AddFile($"file{i}.invalid", stream);
         }
 

tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/PhysicalFileSystemTreeServiceTests.cs

@@ -0,0 +1,95 @@
+using System.IO;
+using Microsoft.Extensions.Logging;
+using NUnit.Framework;
+using Umbraco.Cms.Api.Management.Services.FileSystem;
+using Umbraco.Cms.Api.Management.ViewModels.Tree;
+using Umbraco.Cms.Core.IO;
+
+namespace Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees;
+
+public class PhysicalFileSystemTreeServiceTests : FileSystemTreeServiceTestsBase
+{
+    protected override string FileExtension { get; set; } = string.Empty;
+
+    protected override string FileSystemPath => "/";
+
+    protected override void CreateFiles()
+    {
+        var paths = new[]
+        {
+            Path.Join("App_Plugins", "test-extension", "test.js"),
+            Path.Join("wwwroot", "css", "test.css"),
+            Path.Join("wwwroot", "css", "test2.css"),
+            Path.Join("wwwroot", "css", "test3.css"),
+            Path.Join("wwwroot", "css", "test4.css"),
+            Path.Join("Program.cs"),
+        };
+        foreach (var path in paths)
+        {
+            var stream = CreateStream();
+            TestFileSystem.AddFile(path, stream);
+        }
+    }
+
+    [Test]
+    public void Can_Get_Siblings()
+    {
+        var service = CreateService();
+
+        FileSystemTreeItemPresentationModel[] treeModels = service.GetSiblingsViewModels("wwwroot/css/test2.css", 1, 1, out long before, out var after);
+
+        Assert.AreEqual(3, treeModels.Length);
+        Assert.AreEqual(treeModels[0].Name, "test.css");
+        Assert.AreEqual(treeModels[1].Name, "test2.css");
+        Assert.AreEqual(treeModels[2].Name, "test3.css");
+        Assert.AreEqual(before, 0);
+        Assert.AreEqual(after, 1);
+    }
+
+    [Test]
+    public void Can_Get_Ancestors()
+    {
+        var service = CreateService();
+
+        FileSystemTreeItemPresentationModel[] treeModels = service.GetAncestorModels(Path.Join("wwwroot", "css", "test.css"), true);
+
+        Assert.IsNotEmpty(treeModels);
+        Assert.AreEqual(treeModels.Length, 3);
+        Assert.AreEqual(treeModels[0].Name, "wwwroot");
+        Assert.AreEqual(treeModels[1].Name, "css");
+        Assert.AreEqual(treeModels[2].Name, "test.css");
+    }
+
+    [Test]
+    public void Can_Get_Root_PathViewModels()
+    {
+        var service = CreateService();
+
+        FileSystemTreeItemPresentationModel[] treeModels = service.GetPathViewModels(string.Empty, 0, int.MaxValue, out var totalItems);
+
+        Assert.IsNotEmpty(treeModels);
+        Assert.AreEqual(totalItems, 2);
+        Assert.AreEqual(treeModels.Length, totalItems);
+        Assert.AreEqual(treeModels[0].Name, "App_Plugins");
+        Assert.AreEqual(treeModels[1].Name, "wwwroot");
+    }
+
+    [Test]
+    public void Can_Get_Child_PathViewModels()
+    {
+        var service = CreateService();
+
+        FileSystemTreeItemPresentationModel[] treeModels = service.GetPathViewModels("App_Plugins/test-extension", 0, int.MaxValue, out var totalItems);
+
+        Assert.IsNotEmpty(treeModels);
+        Assert.AreEqual(totalItems, 1);
+        Assert.AreEqual(treeModels.Length, totalItems);
+        Assert.AreEqual(treeModels[0].Name, "test.js");
+    }
+
+    private PhysicalFileSystemTreeService CreateService()
+    {
+        var physicalFileSystem = new PhysicalFileSystem(IOHelper, HostingEnvironment, LoggerFactory.CreateLogger<PhysicalFileSystem>(), HostingEnvironment.MapPathWebRoot(FileSystemPath), HostingEnvironment.ToAbsolute(FileSystemPath));
+        return new PhysicalFileSystemTreeService(physicalFileSystem);
+    }
+}

tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/ScriptTreeServiceTests.cs

@@ -8,12 +8,13 @@ namespace Umbraco.Cms.Tests.Integration.ManagementApi.Services.Trees;
 public class ScriptTreeServiceTests : FileSystemTreeServiceTestsBase
 {
     protected override string FileExtension { get; set; } = ".js";
+
     protected override string FileSystemPath => GlobalSettings.UmbracoScriptsPath;
 
     protected override IFileSystem? GetScriptsFileSystem() => TestFileSystem;
 
     [Test]
-    public void Can_Get_Siblings_From_Script_Tree_Service()
+    public void Can_Get_Siblings()
     {
         var service = new ScriptTreeService(FileSystems);
 
@@ -29,20 +30,20 @@ public class ScriptTreeServiceTests : FileSystemTreeServiceTestsBase
     }
 
     [Test]
-    public void Can_Get_Ancestors_From_StyleSheet_Tree_Service()
+    public void Can_Get_Ancestors()
     {
         var service = new ScriptTreeService(FileSystems);
 
         var path = Path.Join("tests", $"file5{FileExtension}");
-        FileSystemTreeItemPresentationModel[] treeModel = service.GetAncestorModels(path, true);
+        FileSystemTreeItemPresentationModel[] treeModels = service.GetAncestorModels(path, true);
 
-        Assert.IsNotEmpty(treeModel);
+        Assert.IsNotEmpty(treeModels);
-        Assert.AreEqual(treeModel.Length, 2);
+        Assert.AreEqual(treeModels.Length, 2);
-        Assert.AreEqual(treeModel[0].Name, "tests");
+        Assert.AreEqual(treeModels[0].Name, "tests");
     }
 
     [Test]
-    public void Can_Get_PathViewModels_From_StyleSheet_Tree_Service()
+    public void Can_Get_PathViewModels()
     {
         var service = new ScriptTreeService(FileSystems);
 
@@ -58,7 +59,7 @@ public class ScriptTreeServiceTests : FileSystemTreeServiceTestsBase
         var service = new ScriptTreeService(FileSystems);
         for (int i = 0; i < 2; i++)
         {
-            using var stream = CreateStream(Path.Join("tests"));
+            using var stream = CreateStream();
             TestFileSystem.AddFile($"file{i}.invalid", stream);
         }
 

tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/StyleSheetTreeServiceTests.cs

@@ -14,7 +14,7 @@ public class StyleSheetTreeServiceTests : FileSystemTreeServiceTestsBase
     protected override IFileSystem? GetStylesheetsFileSystem() => TestFileSystem;
 
     [Test]
-    public void Can_Get_Siblings_From_StyleSheet_Tree_Service()
+    public void Can_Get_Siblings()
     {
         var service = new StyleSheetTreeService(FileSystems);
 
@@ -30,20 +30,20 @@ public class StyleSheetTreeServiceTests : FileSystemTreeServiceTestsBase
     }
 
     [Test]
-    public void Can_Get_Ancestors_From_StyleSheet_Tree_Service()
+    public void Can_Get_Ancestors()
     {
         var service = new StyleSheetTreeService(FileSystems);
 
         var path = Path.Join("tests", $"file5{FileExtension}");
-        FileSystemTreeItemPresentationModel[] treeModel = service.GetAncestorModels(path, true);
+        FileSystemTreeItemPresentationModel[] treeModels = service.GetAncestorModels(path, true);
 
-        Assert.IsNotEmpty(treeModel);
+        Assert.IsNotEmpty(treeModels);
-        Assert.AreEqual(treeModel.Length, 2);
+        Assert.AreEqual(treeModels.Length, 2);
-        Assert.AreEqual(treeModel[0].Name, "tests");
+        Assert.AreEqual(treeModels[0].Name, "tests");
     }
 
     [Test]
-    public void Can_Get_PathViewModels_From_StyleSheet_Tree_Service()
+    public void Can_Get_PathViewModels()
     {
         var service = new StyleSheetTreeService(FileSystems);
 
@@ -59,7 +59,7 @@ public class StyleSheetTreeServiceTests : FileSystemTreeServiceTestsBase
         var service = new StyleSheetTreeService(FileSystems);
         for (int i = 0; i < 2; i++)
         {
-            using var stream = CreateStream(Path.Join("tests"));
+            using var stream = CreateStream();
             TestFileSystem.AddFile($"file{i}.invalid", stream);
         }