Make WordPress Core

Opened 6 weeks ago

Last modified 12 days ago

#64779 new defect (bug)

Notes can be edited and deleted by other users

Reported by: mindctrl's profile mindctrl Owned by:
Milestone: 7.1 Priority: normal
Severity: normal Version: 6.9
Component: Comments Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

A user with access to edit a post is able to edit the Notes of other users on that post. This happens because the edit_comment capability gets mapped to edit_post.

Example

A user with the Contributor role creates a new draft post. A user with the Administrator role leaves a Note on the post that says "Please change this to...". The Contributor user edits the Note to say "Looks good to me". Another editor/admin comes along later, sees the note, and publishes the post, believing the Note content is original.

It's also possible for the user with the Contributor role to delete the Note left by the admin user.

Notes

This mirrors the existing behavior for normal comments, but it feels like the behavior for Notes should be different considering they're used for collaboration where the history of the conversation helps understand the evolution of the content, and the mutability could lead to operational impacts.

Attachments (1)

64779-notes-cap-fix.diff (1.1 KB) - added by sachinrajcp123 6 weeks ago.
Attached patch for review.

Download all attachments as: .zip

Change History (8)

#1 @westonruter
6 weeks ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 7.0

It does seem like users who don't have the manage_comments capability shouldn't be able to edit notes which they didn't author themselves.

#2 @adamsilverstein
6 weeks ago

cc: @Mamaduka

@sachinrajcp123
6 weeks ago

Attached patch for review.

This ticket was mentioned in PR #11191 on WordPress/wordpress-develop by @mindctrl.


5 weeks ago
#3

  • Keywords has-patch has-unit-tests added; needs-patch removed

Notes (internal collaborative comments on posts) currently inherit the same capability mapping as regular comments, meaning anyone who can edit the parent post can edit or delete any note on it.

Trac ticket: https://core.trac.wordpress.org/ticket/64779

## Use of AI Tools
Opus 4.6 helped with this, particularly the phpunit tests.

#4 @mindctrl
5 weeks ago

  • Keywords needs-testing added

The existing patch here is incomplete, but also uses the incorrect capability name.

I've opened a new PR for this here: https://github.com/WordPress/wordpress-develop/pull/11191

#5 @ozgursar
5 weeks ago

  • Keywords needs-testing removed

Patch Testing Report

Patch Tested: https://github.com/WordPress/wordpress-develop/pull/11191

Environment

  • WordPress: 7.0-beta4-61919-src
  • PHP: 8.2.29
  • Server: nginx/1.29.4
  • Database: mysqli (Server: 8.4.7 / Client: mysqlnd 8.2.29)
  • Browser: Opera
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.4
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.1

Steps taken

  1. Create a test user with Contributor role
  2. Create a draft post using the test user
  3. Add a note on the draft post using the admin user
  4. Try to change that note using the test user and confirm that note can be successfully changed with the message Note updated.
  5. Apply patch
  6. Try to edit the admin user's comment again
  7. Confirm that Sorry, you are not allowed to edit this comment. is displayed when updated.
  8. ✅ Patch is solving the problem

Expected result

  • Users who don't have the manage_comments capability shouldn't be able to edit notes which they didn't author themselves.

Screenshots/Screencast with results

Before
https://i.imgur.com/9eYFpWS.png

After
https://i.imgur.com/2fQno3v.png

#6 @audrasjb
3 weeks ago

  • Milestone changed from 7.0 to 7.1

Since the PR is still a draft with change requests and as we're close to 7.0 RC1, I'm moving this to 7.1.

#7 @manfcarlo
12 days ago

I think the pre-existing behaviour (any user able to delete any note) is correct. If someone leaves a note with profanity or personal attacks, or a typo, it should be easy for others to delete/correct without having to contact someone with higher privileges. If someone is maliciously deleting or changing other users' notes, they would be better off having their editing access to the whole post revoked.

Note: See TracTickets for help on using tickets.