Release backport - #1714
Conversation
Any logged-in user could call edac_insert_ignore_data to dismiss issues
on posts they cannot edit. Add current_user_can('edit_post') check using
the same postid lookup pattern as the REST /dismiss-issue endpoint.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The initial fix only checked the first issue ID. A mixed-ID batch or a largeBatch request (which updates by object string site-wide) could still affect posts the user cannot edit. Now resolves all distinct postids for the full batch (or all posts sharing the same object in largeBatch mode) and gates on edit_post for every one before any UPDATE runs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The SELECT object query was running twice for largeBatch requests — once during the capability check and again in the execution block. Reuse $batch_object to avoid the redundant query. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Return -2 error when $affected_post_ids is empty (stale IDs should not succeed silently) rather than letting the foreach no-op and responding success - Remove redundant (array) cast; wpdb::get_col() always returns an array Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WP 7.0 added an `instanceof WP_Screen` guard to `get_current_screen()` (wp-admin/includes/screen.php). The anonymous-class mocks assigned to $GLOBALS['current_screen'] in EnqueueAdminTest and MetaBoxesTest no longer satisfy that check, so `Helpers::is_block_editor()` short-circuits to false and several enqueue/metabox tests fail. `WP_Screen` is final, so we can't subclass it. Use `set_current_screen()` to install a real screen and toggle the public `is_block_editor` property to simulate block-editor vs classic-editor contexts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… side effects set_current_screen() sets $hook_suffix, $typenow, and $taxnow, and fires the current_screen action. Since tearDown() only unsets $GLOBALS['current_screen'], those globals were left polluted between tests. WP_Screen::get() returns a proper WP_Screen instance (satisfying the WP 7.0 instanceof check) without any of those side effects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tests: fix screen mocks for WP 7.0 (instanceof WP_Screen)
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughVersion 1.42.1 patch release adds permission validation to the ignore AJAX endpoint, ensuring users can only affect posts they have edit permission for. Version metadata updated across all files. Test screen mocking refactored to use real WP_Screen instances for WordPress 7.0 compatibility. ChangesVersion 1.42.1 Patch Release
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Accessibility Checker plugin to version 1.42.1, primarily focusing on enhancing security by implementing granular permission checks in the add_ignore AJAX action. It also updates test mocks to ensure compatibility with WordPress 7.0. Review feedback suggests optimizing performance by priming post caches before iterating through affected posts and recommends using the validated table name variable in database queries for better consistency and safety.
| foreach ( $affected_post_ids as $affected_post_id ) { | ||
| if ( ! current_user_can( 'edit_post', (int) $affected_post_id ) ) { | ||
| wp_send_json_error( new \WP_Error( '-5', __( 'Permission Denied', 'accessibility-checker' ) ) ); | ||
| } | ||
| } |
There was a problem hiding this comment.
To improve performance when checking permissions for multiple posts, it is recommended to prime the post caches before the loop. This prevents current_user_can() from triggering a separate database query for each post that isn't already in the cache, which is especially important if the affected_post_ids list is large. Additionally, we use a strict is_array() check to ensure the variable is an array before processing, adhering to repository standards.
if ( is_array( $affected_post_ids ) ) {
_prime_post_caches( array_map( 'intval', $affected_post_ids ) );
foreach ( $affected_post_ids as $affected_post_id ) {
if ( ! current_user_can( 'edit_post', (int) $affected_post_id ) ) {
wp_send_json_error( new \WP_Error( '-5', __( 'Permission Denied', 'accessibility-checker' ) ) );
}
}
}References
- When ensuring a variable is an array, prefer using an if...then conditional check over (array) type casting to maintain consistency with existing code patterns.
- When the contract for a variable is a specific type (e.g., an array of commands), prefer a stricter type check (e.g., is_array) over a more general one (e.g., is_iterable) to enforce that contract.
| // $batch_object was already resolved during the capability check above. | ||
| $object = $batch_object; | ||
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Safe variable used for table name, caching not required for one time operation. | ||
| $wpdb->query( $wpdb->prepare( 'UPDATE %i SET ignre = %d, ignre_user = %d, ignre_date = %s, ignre_comment = %s, ignre_reason = %s, ignre_global = %d WHERE siteid = %d and object = %s', $table_name, $ignre, $ignre_user, $ignre_date, $ignre_comment, $ignre_reason, $ignore_global, $siteid, $object ) ); |
There was a problem hiding this comment.
For consistency and to ensure the use of the validated table name, please use $valid_table instead of $table_name in this query. The $valid_table variable was already verified using edac_get_valid_table_name() earlier in the function.
$wpdb->query( $wpdb->prepare( 'UPDATE %i SET ignre = %d, ignre_user = %d, ignre_date = %s, ignre_comment = %s, ignre_reason = %s, ignre_global = %d WHERE siteid = %d and object = %s', $valid_table, $ignre, $ignre_user, $ignre_date, $ignre_comment, $ignre_reason, $ignore_global, $siteid, $object ) );
backport
Summary by CodeRabbit