-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Static files: Fix tree to only provide items from expected folders (closes #20962) #21001
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Static files: Fix tree to only provide items from expected folders (closes #20962) #21001
Conversation
There was a problem hiding this 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
PhysicalFileSystemTreeServiceto restrict access toApp_Pluginsandwwwrootfolders only - Added comprehensive integration tests for the
PhysicalFileSystemTreeServiceto 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 |
Prerequisites
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_Pluginsandwwwrootwas 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
mainthat needs to be resolved.Should return:
And not any other folders found at the root of the application.
Requests such as the following for child items should also work: