Honor REST access controls for protected block content by maxschmeling · Pull Request #118 · Automattic/vip-block-data-api

maxschmeling · 2026-09-25T20:56:15Z

Description

Prevents the Block Data API REST endpoint from returning raw block content that WordPress does not permit the current user to read.

Delegates item authorization to each post type's registered REST controller instead of copying the base posts-controller logic.
Requires edit capability before returning password-protected post_content.
Preserves vip_block_data_api__rest_validate_post_id as an explicit override point for integrations that provide their own authorization logic.
Matches WordPress core's type, status, and password checks when expanding synced patterns embedded in otherwise public content.
Documents the tightened access-control contract.

Steps to Test

Check out this PR and install Composer dependencies.
Start wp-env.
Run the PHPUnit suite in single-site mode.
Run the PHPUnit suite with WP_MULTISITE=1.
Run composer phpcs.
Confirm anonymous requests cannot retrieve a password-protected post or standalone wp_block, while public posts and authorized editor requests continue to work.

Verification

WordPress 7.1.2 single-site: 97 tests, 246 assertions.
WordPress 7.1.2 multisite: 97 tests, 246 assertions.
Focused REST and synced-pattern security tests: 27 tests, 90 assertions.
PHPCS passes.
PHP syntax checks pass for all changed PHP files.

ingeniumed

I have two changes I think this PR should make, plus one access-control boundary worth clarifying:

Denied, password-protected, and missing posts currently return 400 rest_invalid_param. The access check should run in the route’s permission callback so it can return an appropriate 401/403 or 404. Tests should confirm that these error responses contain no protected content.
The new call to a post type’s REST controller should handle an exception during controller construction or get_item_permissions_check(). It should fail closed with a generic error and never continue to parse the post. A throwing test controller would verify this.
Does this endpoint intentionally return raw post_content when a custom controller permits the item but removes content from its REST response? Response redaction does not change the stored post content that this plugin parses. The password check handles WordPress’s known case, and I have not found a custom controller here that does the same thing. It would help to state
the intended behavior so the PR does not imply that checking item permissions also honors response redaction.

A REST test through a public post referencing an inaccessible synced pattern would also verify that the pattern’s content cannot appear in an otherwise readable parent response.

ingeniumed · 2026-09-27T22:18:15Z

 						 * @param int $post_id The queried post ID.
 						 */
-						return apply_filters( 'vip_block_data_api__rest_validate_post_id', $is_valid, $post_id );
+						$is_allowed = apply_filters( 'vip_block_data_api__rest_validate_post_id', $is_readable, $post_id );


This is a breaking change for those customers that depended on this behaviour.

This should be noted in a breaking changes section.

@maxschmeling I think we should keep this behavior the same as before if possible. The vip_block_data_api__rest_validate_post_id filter existed to give developers an option to expose a post that our default logic blocked. If a developer wants to use special logic to expose a post in their own code using special rules (e.g. meta capabilities, auth token) I don't think we should block it.

Isn't this is the code path to implementing preview functionality via this API?

Max rolled this back in 99cbf95, so the filter can still override default as I think it should work.

ingeniumed · 2026-09-27T22:50:27Z

-						return apply_filters( 'vip_block_data_api__rest_validate_post_id', $is_valid, $post_id );
+						$is_allowed = apply_filters( 'vip_block_data_api__rest_validate_post_id', $is_readable, $post_id );
+
+						return $is_readable && $is_allowed;


Denied, password-protected, and missing posts all become 400 rest_invalid_param because access is checked during ID validation. Could this check move to permission_callback() so the route returns 401/403 for denied access and 404 for a missing post? The tests should also confirm that error responses contain no protected content.

I don't mind the check in the ID parameter. If a post shouldn't exist in the public API yet, different status responses to indicate an invalid post ID leaks some information, e.g. "Post XXX exists but is password protected, or otherwise inaccessible" is more information that "Post XXX isn't valid".

As mentioned above, I think the best way to be consistent and avoid leaking information is to validate everything based on the ID OR permission_callback, but probably not both. I think reporting a missing ID and a draft/password protected ID the same way in this validation is fine. I left the check here for now, let me know if you disagree.

ingeniumed · 2026-09-27T22:50:54Z

-		// Use parent status if inheriting.
-		if ( 'inherit' === $post->post_status && $post->post_parent > 0 ) {
-			return self::is_post_readable( $post->post_parent );
+		$rest_controller = $post_type->get_rest_controller();


A custom controller can throw during construction or in get_item_permissions_check(). Since this route now calls that controller directly, it should handle either failure without parsing the post or exposing exception details. A throwing test controller would verify a generic error response.

Added a try/catch and tests in 30793b1, good idea.

ingeniumed · 2026-09-27T22:51:45Z

-		}
+		// Use the registered controller rather than copying the base posts controller's
+		// permission logic. Post types such as wp_block add stricter checks in subclasses.
+		$request = new WP_REST_Request( 'GET' );


One boundary worth clarifying: if a custom controller permits an item but removes its content from the REST response, should GET /vip-block-data-api/v1/posts/{id}/blocks still return blocks parsed from stored post_content? The permission check does not run response preparation

This is tricky. If we parse content from the direct REST response (possible fix for this issue), we even more deeply rely on the internal structure of the REST controller which can be filtered and changed a lot of ways. Using the REST security model is simple get_item_permissions_check(), but I'm worried about becoming too dependent on other REST internals like this.

I think for now I'd prefer returning blocks parsed from post_content for consistency if a user has read access to the item according to the controller, even if the response is filtered or changed.

@ingeniumed I added a comment to the README about the security modal and how we use post_content directly in fae2906, so at least it should be easy to find this information if content isn't working as expected.

…ccess on failure

alecgeatches · 2026-10-07T19:32:04Z

Max's changes and my commits should have addressed the comments above, please have another review when you have a chance. Thanks!

Fix REST access controls for protected content

6c6633a

maxschmeling requested a review from a team as a code owner September 25, 2026 20:56

ingeniumed requested changes Sep 27, 2026

View reviewed changes

ingeniumed requested a review from alecgeatches September 27, 2026 22:54

maxschmeling and others added 4 commits September 28, 2026 15:46

Restore REST validation filter overrides

99cbf95

Wrap REST controller check in try/catch, log error and do not allow a…

30793b1

…ccess on failure

Add README clarifications about security change

fae2906

Add additional tests for wp_block and password-protected posts

6326764

alecgeatches requested a review from ingeniumed October 7, 2026 19:31

chriszarate approved these changes Oct 7, 2026

View reviewed changes

Merge branch 'trunk' into fix/vipcms-2342-protected-content

9928675

alecgeatches approved these changes Oct 7, 2026

View reviewed changes

ingeniumed approved these changes Oct 7, 2026

View reviewed changes

alecgeatches merged commit 5ce64f8 into trunk Oct 7, 2026
20 checks passed

alecgeatches deleted the fix/vipcms-2342-protected-content branch October 7, 2026 20:19

alecgeatches mentioned this pull request Oct 7, 2026

Release 1.4.12 #121

Merged

Conversation

maxschmeling commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Steps to Test

Verification

Uh oh!

ingeniumed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alecgeatches Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alecgeatches Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alecgeatches commented Oct 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maxschmeling commented Sep 25, 2026 •

edited

Loading

alecgeatches Sep 28, 2026 •

edited

Loading

alecgeatches Oct 7, 2026 •

edited

Loading