Skip to content

Check the attachment, not only the role, on the media library thumbnail and EXIF routes - #2247

Open
subodhr258 wants to merge 1 commit into
developfrom
fix/media-library-route-permissions
Open

subodhr258 wants to merge 1 commit into
developfrom
fix/media-library-route-permissions

Conversation

@subodhr258

Copy link
Copy Markdown
Collaborator

What changes

The routes behind the Media Editor's thumbnail picker and the Media Library's EXIF panel asked only for edit_posts, which says nothing about whose media a request acts on. They now check the attachment named in the request (inc/classes/rest-api/class-media-library.php):

Route Now needs
set-video-thumbnail, upload-custom-video-thumbnail, delete-custom-video-thumbnail edit access to that video (can_edit_attachment)
get-video-thumbnail, get-exif-data read access to that attachment, or upload_files for media the Media Library already lists to that user (can_read_attachment). A private attachment still needs WordPress's own read access

Also:

  • get-video-thumbnail saves the thumbnails it fetched from GoDAM Central, and repairs a stale poster, only when the caller may edit the video. Other callers get the same response without a write.
  • Thumbnail URLs must be absolute http or https URLs (sanitize_thumbnail_url()); custom thumbnail URLs were not checked before.
  • Removing a video's last custom thumbnail now also clears it as the selected poster when it was the selected one (it used to stay pointing at the removed image).
  • Removing one of several custom thumbnails keeps the stored list a list, so the next read returns a JSON array. Before, the gap in the keys made it a JSON object, which the picker can't show.

No change: the folder ZIP (download-folder) and GoDAM Central listing (get-godam-cmm-files) routes keep their upload_files check. Users who pass it can already list and open every item in the Media Library, so those routes show them nothing more. Tests pin that.

Testing

Throwaway WordPress (Playground) with the 2.5.0 release plus this file; Admin, Editor, two Authors (A owns the media), Contributor and Subscriber, each signed in over HTTP with a REST nonce; GoDAM Central stubbed.

Check Result
Author B or a Contributor selects, adds or removes a thumbnail on Author A's video 403
Author A, Editor, Admin do the same 200, as before
A non-http(s) thumbnail URL 400
Contributor reads thumbnails or EXIF of media on a draft post, or a private attachment 403 (WordPress's media API refuses the same)
Author B reads EXIF of a public image 200, as before
Thumbnail read by someone who can't edit the video same response, nothing written
Removing the last custom thumbnail poster cleared
Media Editor picker flow (read, select, add, remove) for the owner, an Editor and Admin works
EXIF panel, folder ZIP, GoDAM tab for Authors work as before
  • PHPUnit: 38 new tests in tests/php/MediaLibraryAttachmentAccessTest.php; full suite OK (651 tests).
  • phpcs clean; bin/hook-check checks pass.

Worth knowing

  • An Author who clicks a thumbnail on another author's video in the Media Library now gets a 403. The picker doesn't show the error yet; WordPress itself doesn't let that Author edit the item either.
  • A missing attachment_id now returns 403 instead of 404.
  • Relative or protocol-relative thumbnail URLs are now refused.
  • No host allowlist for thumbnail URLs: GoDAM's CDN host is assigned by Central and offload plugins use their own hosts, so a list would break the picker on those sites.

QA:

  1. As the owner of a video, open the Media Editor's thumbnail picker: select a generated thumbnail, add a custom one, remove it. All work.
  2. As another Author, try to change the thumbnail of someone else's video: refused.
  3. Open an image's details in the Media Library: EXIF shows.
  4. Add two custom thumbnails, remove one, reopen the picker: the other still shows.

Issue: rtCamp/godam-plugin-wp#76

…il and EXIF routes

The thumbnail routes behind the Media Editor's picker and the EXIF route
behind the Media Library details panel asked only for edit_posts, which does
not say whose media a request acts on. They now check the attachment named in
the request:

- Selecting a poster, adding a custom thumbnail and removing one need edit
  access to the video.
- Reading a video's thumbnails or an image's EXIF data needs read access to
  the attachment, or the right to upload files, because the Media Library
  lists every attachment to those users. A private attachment still needs
  WordPress's own read access.
- The thumbnail read saves the thumbnails it fetched from Central only when
  the caller may edit the video. Other callers get the same response without
  a write.
- A thumbnail URL has to be an absolute http(s) URL.
- Removing a video's last custom thumbnail also clears it as the selected
  poster when it was the selected one.
- Removing one of several custom thumbnails keeps the stored list a list, so
  the next read returns a JSON array the picker can show.

The folder ZIP and Central listing routes keep their upload_files check. Users
who pass it can already list every item in the Media Library and open its
file, so those routes show them nothing more. Tests pin that.
@subodhr258 subodhr258 self-assigned this Oct 23, 2026
@github-actions

Copy link
Copy Markdown

🔍 WordPress Plugin Check Report

⚠️ Status: Passed with warnings

📊 Report

🎯 Total Issues ❌ Errors ⚠️ Warnings
13 0 13

⚠️ Warnings (13)

📁 readme.txt (2 warnings)
📍 Line 🔖 Check 💬 Message
0 mismatched_plugin_name Plugin name "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" is different from the name declared in plugin header "GoDAM".
0 trademarked_term The plugin name includes a restricted term. Your chosen plugin name - "GoDAM - Organize WordPress Media Library & File Manager with Unlimited Folders for Images, Videos & more" - contains the restricted term "wordpress" which cannot be used at all in your plugin name.
📁 assets/build/css/main.css (1 warning)
📍 Line 🔖 Check 💬 Message
0 EnqueuedStylesScope This style is being loaded in all contexts.
📁 assets/src/libs/analytics.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/23/hello-world/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle analytics-library) is loaded in the footer. Consider a defer or async script loading strategy instead.
📁 assets/build/js/main.min.js (5 warnings)
📍 Line 🔖 Check 💬 Message
0 EnqueuedScriptsScope This script is being loaded in all frontend contexts.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880 (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/2026/10/23/hello-world/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/sample-page/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.
0 NonBlockingScripts.NoStrategy This script on http://localhost:8880/demo-attachment-post/ (with handle rtgodam-script) is loaded in the footer. Consider a defer or async script loading strategy instead.

🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

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.

1 participant