Skip to content

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

Merged
Zeegaan merged 7 commits intomainfrom
v17/bugfix/apply-root-folders-to-static-file-tree
Dec 2, 2025
Merged

Static files: Fix tree to only provide items from expected folders (closes #20962)#21001
Zeegaan merged 7 commits intomainfrom
v17/bugfix/apply-root-folders-to-static-file-tree

Conversation

@AndyButland
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Fixes: #20962

Description

With some recent refactoring to introduce services to back the various file system trees, the filter to only provide items from App_Plugins and wwwroot was lost, leading to the linked issue.

This PR reintroduces that.

Testing

Testing will currently have to be done via the management API as there seems to be a front-end regression since 17.0 in main that needs to be resolved.

GET /umbraco/management/api/v1/tree/static-file/root?skip=0&take=100

Should return:

{
  "total": 2,
  "items": [
    {
      "name": "App_Plugins",
      "path": "/App_Plugins",
      "parent": {
        "path": "/"
      },
      "isFolder": true,
      "hasChildren": true
    },
    {
      "name": "wwwroot",
      "path": "/wwwroot",
      "parent": {
        "path": "/"
      },
      "isFolder": true,
      "hasChildren": true
    }
  ]
}

And not any other folders found at the root of the application.

Requests such as the following for child items should also work:

GET /umbraco/management/api/v1/tree/static-file/children?parentPath=%2Fwwwroot%2Fassets&skip=0&take=100

Copilot AI review requested due to automatic review settings November 28, 2025 20:41
@AndyButland AndyButland changed the title Static files: Fix tree to only provide items from expected folders Static files: Fix tree to only provide items from expected folders (closes #20962) Nov 28, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a security issue where the static file tree was exposing all folders at the application root instead of only the intended App_Plugins and wwwroot folders. The fix moves the filtering logic from the controller layer into the PhysicalFileSystemTreeService, ensuring that only files and folders within the allowed directories are accessible through the Management API.

Key Changes

  • Moved path filtering logic to PhysicalFileSystemTreeService to restrict access to App_Plugins and wwwroot folders only
  • Added comprehensive integration tests for the PhysicalFileSystemTreeService to verify filtering behavior
  • Refactored test helper methods and naming conventions for better consistency across test files

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Umbraco.Cms.Api.Management/Services/FileSystem/PhysicalFileSystemTreeService.cs Implements path filtering logic to only allow access to App_Plugins and wwwroot folders
src/Umbraco.Cms.Api.Management/Services/FileSystem/FileSystemTreeServiceBase.cs Makes GetDirectories and GetFiles methods virtual to allow override in derived classes
src/Umbraco.Cms.Api.Management/Controllers/StaticFile/Tree/StaticFileTreeControllerBase.cs Adds override keyword to GetDirectories and GetFiles methods for backward compatibility with obsolete constructor path
tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/PhysicalFileSystemTreeServiceTests.cs Adds comprehensive tests for PhysicalFileSystemTreeService including security filtering validation
tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/FileSystemTreeServiceTestsBase.cs Refactors CreateStream helper to remove unused parameter and extracts CreateFiles to separate method
tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/StyleSheetTreeServiceTests.cs Simplifies test names and variable naming, updates CreateStream usage
tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/ScriptTreeServiceTests.cs Simplifies test names and variable naming, updates CreateStream usage
tests/Umbraco.Tests.Integration/ManagementApi/Services/Trees/PartialViewTreeServiceTests.cs Simplifies test names and variable naming, updates CreateStream usage

Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good and tests good, just have a single comment about the breaking change 🫡

@AndyButland AndyButland requested a review from Zeegaan December 1, 2025 11:10
Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 💪

@Zeegaan Zeegaan merged commit 84c15ff into main Dec 2, 2025
26 checks passed
@Zeegaan Zeegaan deleted the v17/bugfix/apply-root-folders-to-static-file-tree branch December 2, 2025 01:20
Zeegaan pushed a commit that referenced this pull request Dec 2, 2025
…loses #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.

(cherry picked from commit 84c15ff)
@Zeegaan
Copy link
Member

Zeegaan commented Dec 2, 2025

Cherry picked for 17.0.1 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Able to insert file types which should not be allowed as a thumbnail

2 participants