Confine Media Manager and image resizer paths to their roots

[LukeTowers]

The Media Manager's thumbnail handlers now run every path they are given through MediaLibrary::validatePath(), the rule the rest of the widget and the media library already apply, so no handler in the widget is held to a looser one than its neighbours. The image resizer matches a path against its configured sources on a path boundary and accepts only paths that stay inside the source folder it matched. Crop & Insert keeps the folder and file name it was asked for, takes the stored file's extension from the image the resizer produced, and holds the destination to cms.fileDefinitions.defaultExtensions, the same extension allowlist that uploading and renaming a media item already use.

Thumbnail validation lives inside generateThumbnail(), next to the widget's existing broken-thumbnail handling, rather than in the handler above it. The browser requests thumbnails in batches, so an item the media library will not accept renders that one item's broken thumbnail and leaves the rest of the batch alone.

Behaviour changes

• A crop destination always uses the extension of the image the resizer produced; a requested extension is ignored, so cropping to a different extension than the source's no longer works.
• A crop destination is checked against cms.fileDefinitions.defaultExtensions. Out of the box that list covers every format the resizer can produce, so nothing changes; a site that has narrowed it will find crops to the formats it removed refused, the way uploading and renaming them already are.
• An empty, whitespace-only or non-string crop destination, and a crop source that is not a URL string, are now an Invalid input data error rather than a PHP error.
• The batch thumbnail handler returns the widget's broken thumbnail, rather than a generated one, for a media item whose name falls outside the media library's own path rules. The Media Manager's uploader cannot produce such a name, and the sidebar preview, rename, move and crop already refuse one. The item grid builds its thumbnails while the page renders rather than through this handler, so a folder of such files looks the same as it did before.
• The image resizer resolves a source path only while it stays inside the configured source folder it matched, and matches those sources on folder boundaries. |resize and the image list column fall back to the value they were given, as they already do for an image the resizer cannot identify; |imageWidth and |imageHeight report the failure, as they already do for the same.

Tests

• System\Tests\Classes\ImageResizerTest::testRefusesPathsOutsideTheMatchedSource, ::testRefusesARelativeSegmentThatWouldHaveStayedInsideTheSource, ::testAcceptsPathsInsideTheMatchedSource
• Backend\Tests\Widgets\MediaManagerThumbnailPathTest, including ::testOneRefusedPathInABatchStillThumbnailsTheRestOfTheBatch and ::testAnItemOutsideTheLibraryPathRulesOnlyLosesItsOwnThumbnail
• Backend\Tests\Widgets\MediaManagerCropDestinationTest, including ::testCropRefusesASourceThatIsNotAUrl and ::testTheStockExtensionAllowlistCoversEveryFormatTheResizerProduces

19 of the 26 Media Manager tests and 11 of the 25 resizer tests fail without the production changes in this PR. modules/backend is green at 354 tests / 864 assertions and modules/system at 364 / 1933, both on PHP 8.4 locally; phpcs is clean on the changed files.

Summary by CodeRabbit

• Bug Fixes
  • Thumbnail generation and image cropping now reject paths that escape the media library or source folder, including traversal-style and malformed paths.
  • Cropped images retain their actual output format, even when it differs from the requested file extension.
  • Invalid thumbnail paths use the existing broken-thumbnail fallback, while valid media paths—including nested folders and names with spaces or non-Latin characters—continue to work.
  • A refused path in a batch no longer prevents other valid items from being processed.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 47ba9f10-629a-47f3-b3b4-6239d1a86e7e

📥 Commits

Reviewing files that changed from the base of the PR and between 4ab9e5a and 5e9ace0.

📒 Files selected for processing (5)

• modules/backend/tests/widgets/MediaManagerCropDestinationTest.php
• modules/backend/tests/widgets/MediaManagerThumbnailPathTest.php
• modules/backend/widgets/MediaManager.php
• modules/system/classes/ImageResizer.php
• modules/system/tests/classes/ImageResizerTest.php

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

ImageResizer now requires source-path boundary matches and rejects matched paths with traversal segments. MediaManager validates thumbnail paths and crop inputs. Crop destinations use the produced image format and must pass file-type validation. New tests cover rejected paths and inputs, batch thumbnail behavior, and accepted thumbnail and crop paths.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 5e9ac

The path-validation and crop-output changes have no identified issue requiring resolution before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5e9ac

The reviewed changes generally tighten path and file-type controls. No introduced security failure was established, but behavior with custom image sources and storage configurations remains uncertain.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The shared URL-to-disk rule can affect images under configured media, resized, theme, plugin, module, upload, and custom sources. The reviewed crop handler’s persistent destination is the media library; the supplied evidence does not establish deployment-wide reachability or tenant scope.

Trust Boundaries and Controls

• observed — The request controls the crop source URL and destination path. The handler validates the destination, while ImageResizer decodes the URL path once, matches it to a configured source on a slash boundary, rejects exact parent segments in the remainder, and checks that the disk path exists.
• inferred — These checks block the tested direct and encoded traversal forms, but the supplied source and tests do not prove containment for every custom overlapping source or storage adapter that might reinterpret residual encoding. No PR-worsened escape path was established.

Resilience and Maintainability Implications

• inferred — The newly possible blocked-type outcome can leave a reusable resizer artifact without a media-library item. The earlier successful crop path also retained a resizer artifact, so this partial state alone does not establish increased attacker-controlled storage exposure.

Hardening Proposals

• proposed — For deployments with custom image sources or storage adapters, verify containment after their path handling and cover overlapping-source and residual-encoding cases in boundary tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 52.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main changes: restricting Media Manager and image resizer paths to their configured roots.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}