Skip to content

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
Copilot finished reviewing on behalf of AndyButland November 28, 2025 20:45
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

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

Labels

None yet

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