Skip to content

Add authorization check on destination post during content import#668

Open
miyanialkesh7 wants to merge 1 commit into
lloc:masterfrom
miyanialkesh7:alkesh-security-fixes
Open

Add authorization check on destination post during content import#668
miyanialkesh7 wants to merge 1 commit into
lloc:masterfrom
miyanialkesh7:alkesh-security-fixes

Conversation

@miyanialkesh7

@miyanialkesh7 miyanialkesh7 commented Jul 22, 2026

Copy link
Copy Markdown

Summary

  • The content-import filter already verified the requesting user could read the source post and validated the nonce, but never checked whether they were allowed to edit the destination post before writing imported content into it.
  • Adds an explicit current_user_can( 'edit_post', $dest_post_id ) check in ContentImporter.php alongside the existing checks.

Scope note

Per review feedback on this PR, I've narrowed it down to just this authorization fix. The other changes that were previously bundled in here (ABSPATH guards, i18n fixes, PHPCS/Plugin Check suppressions) have been split out:

  • ABSPATH guard additions have been dropped entirely — every file they were added to is a pure class/interface definition with no top-level output, so per WordPress/WordPress-Coding-Standards#1850 the check doesn't add real protection there.
  • i18n fixes are now in a separate PR.
  • PHPCS/Plugin Check cleanup is now in a separate PR.

Test plan

  • Verify content import still succeeds for a user with edit_post capability on the destination post
  • Verify content import is now blocked for a user without edit_post capability on the destination post

@miyanialkesh7
miyanialkesh7 requested a review from lloc as a code owner July 22, 2026 11:54
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@lloc

lloc commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Hello @miyanialkesh7, thanks for helping. Please do smaller PRs by grouping them so I can review them more easily. Thanks!

@miyanialkesh7

Copy link
Copy Markdown
Author

Hey @lloc,

There is a checkbox available with every change. So, you can take your time to review the file by file, and if everything is good, then you can easily push those changes to the main/master branch.

Please refer to the screenshot below


image

@lloc

lloc commented Jul 23, 2026

Copy link
Copy Markdown
Owner

How GitHub works is clear to me. ;-) What I want is different, though.

Every PR should have a precise scope. Yours seems to have at least 3, which is problematic whenever I have to revert changes for some reason.

Also, for the ABSPATH check, please have a look here for reference:

WordPress/WordPress-Coding-Standards#1850

A quote from a comment by Juliette:

Actually, the defined() check is essentially a nonsense-check unless the file gives direct output.

The content-import filter already verified the requesting user could
read the source post and validated the nonce, but never checked
whether they were allowed to edit the destination post before writing
imported content into it. Add an explicit current_user_can( 'edit_post',
$dest_post_id ) check alongside the existing checks.
@miyanialkesh7
miyanialkesh7 force-pushed the alkesh-security-fixes branch from 5690792 to dc7616c Compare July 23, 2026 14:32
@miyanialkesh7 miyanialkesh7 changed the title Security hardening, i18n fixes, and WP 7.0 readiness (v3.0.1) Add authorization check on destination post during content import Jul 23, 2026
@miyanialkesh7

Copy link
Copy Markdown
Author

Hi @lloc, thanks for the feedback — I've made both changes:

This PR should now be a small, single-purpose diff. Let me know if there's anything else you'd like adjusted.

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.

2 participants