fix: empty status filter, hidden-field probing, own private posts
Review fix round 2: content-query no longer lets get_posts() fall back to 'publish' when every requested status is capability-gated out (now short-circuits to an empty page). Its field filter shares PostPresenter's hidden-field rule (Pods password type, protected meta) so a hidden key is rejected like an unknown one instead of reaching the query. Password fields are now type-checked as strings on write. A user without edit_private_posts can still query their own private posts when the query is already author-restricted, matching WordPress core. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7
This commit is contained in:
@@ -9,6 +9,7 @@ use LIT\MCP\Abilities\Cursor;
|
||||
use LIT\MCP\Abilities\Errors;
|
||||
use LIT\MCP\Integrations\WordPress\PostGuard;
|
||||
use LIT\MCP\Integrations\WordPress\Schema\ContentSchema;
|
||||
use LIT\MCP\Integrations\WordPress\Schema\FieldDefinition;
|
||||
use WP_Error;
|
||||
use WP_Post;
|
||||
|
||||
@@ -137,8 +138,16 @@ final class ContentQueryAbility implements Ability
|
||||
}
|
||||
}
|
||||
|
||||
$args = $this->queryArgs($input);
|
||||
if ($args['post_status'] === []) {
|
||||
// Every requested status was gated out by the post type's caps – get_posts()
|
||||
// would otherwise treat an empty post_status as its own default ('publish'),
|
||||
// returning results the caller has no right to see instead of none at all.
|
||||
return ['items' => [], 'next_cursor' => null];
|
||||
}
|
||||
|
||||
$limit = Cursor::limit($input['limit'] ?? null);
|
||||
$posts = get_posts(array_merge($this->queryArgs($input), [
|
||||
$posts = get_posts(array_merge($args, [
|
||||
'posts_per_page' => $limit + 1,
|
||||
'offset' => $offset,
|
||||
]));
|
||||
@@ -164,9 +173,18 @@ final class ContentQueryAbility implements Ability
|
||||
$typeObject = get_post_type_object($postType);
|
||||
$caps = is_object($typeObject) && isset($typeObject->cap) ? $typeObject->cap : null;
|
||||
|
||||
// Without edit_others_posts, even a status the user may edit in principle must
|
||||
// stay limited to their own posts – otherwise the query would leak drafts etc.
|
||||
// of other authors that happen to pass the per-status gate below.
|
||||
$restrictedToOwnPosts = ! current_user_can($this->capability($caps, 'edit_others_posts'));
|
||||
|
||||
$args = [
|
||||
'post_type' => $postType,
|
||||
'post_status' => $this->allowedStatuses((array) ($input['status'] ?? self::STATUSES), $caps),
|
||||
'post_status' => $this->allowedStatuses(
|
||||
(array) ($input['status'] ?? self::STATUSES),
|
||||
$caps,
|
||||
$restrictedToOwnPosts
|
||||
),
|
||||
'orderby' => (string) ($input['orderby'] ?? 'date'),
|
||||
'order' => strtoupper((string) ($input['order'] ?? 'desc')),
|
||||
'no_found_rows' => true,
|
||||
@@ -174,10 +192,7 @@ final class ContentQueryAbility implements Ability
|
||||
'suppress_filters' => false,
|
||||
];
|
||||
|
||||
// Without edit_others_posts, even a status the user may edit in principle must
|
||||
// stay limited to their own posts – otherwise the query would leak drafts etc.
|
||||
// of other authors that happen to pass the per-status gate below.
|
||||
if (! current_user_can($this->capability($caps, 'edit_others_posts'))) {
|
||||
if ($restrictedToOwnPosts) {
|
||||
$args['author'] = get_current_user_id();
|
||||
}
|
||||
|
||||
@@ -243,10 +258,15 @@ final class ContentQueryAbility implements Ability
|
||||
* even fetches rows that would only be discarded by the per-post check afterwards
|
||||
* (which would otherwise waste a page slot and reveal the post existed).
|
||||
*
|
||||
* "private" is the one exception: WordPress lets an author edit their own private
|
||||
* post with just edit_posts, so when the query is already author-restricted to the
|
||||
* current user (no edit_others_posts) their own private posts stay in scope even
|
||||
* without edit_private_posts.
|
||||
*
|
||||
* @param list<mixed> $statuses
|
||||
* @return list<string>
|
||||
*/
|
||||
private function allowedStatuses(array $statuses, ?object $caps): array
|
||||
private function allowedStatuses(array $statuses, ?object $caps, bool $restrictedToOwnPosts): array
|
||||
{
|
||||
$gates = [
|
||||
'private' => $this->capability($caps, 'edit_private_posts'),
|
||||
@@ -256,7 +276,17 @@ final class ContentQueryAbility implements Ability
|
||||
|
||||
return array_values(array_filter(
|
||||
array_map('strval', $statuses),
|
||||
static fn (string $status): bool => ! isset($gates[$status]) || current_user_can($gates[$status])
|
||||
static function (string $status) use ($gates, $restrictedToOwnPosts): bool {
|
||||
if (! isset($gates[$status])) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if ($status === 'private' && $restrictedToOwnPosts) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return current_user_can($gates[$status]);
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
@@ -267,7 +297,11 @@ final class ContentQueryAbility implements Ability
|
||||
|
||||
private function rejectUnlessKnownField(string $postType, string $key): ?WP_Error
|
||||
{
|
||||
$allowed = array_keys($this->schema->fields($postType));
|
||||
$visible = array_filter(
|
||||
$this->schema->fields($postType),
|
||||
static fn (FieldDefinition $field): bool => ! PostPresenter::isHiddenField($field)
|
||||
);
|
||||
$allowed = array_keys($visible);
|
||||
if (in_array($key, $allowed, true)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -144,7 +144,7 @@ class PostPresenter
|
||||
{
|
||||
$result = [];
|
||||
foreach ($this->schema->fields((string) $post->post_type) as $key => $field) {
|
||||
if ($this->isHidden($field)) {
|
||||
if (self::isHiddenField($field)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -158,9 +158,11 @@ class PostPresenter
|
||||
* Pods password fields and readonly protected meta (WordPress' own "_"-prefixed
|
||||
* internal keys) are never read out via lit-content-get/-query: neither belongs in
|
||||
* front of the model, and both are one plugin registration away from leaking secrets
|
||||
* or internal engine state.
|
||||
* or internal engine state. Public and static so ContentQueryAbility can apply the
|
||||
* same rule to its `field` filter – a hidden field must be as unreachable through the
|
||||
* query filter as it is through the fields map, or the filter becomes a probe.
|
||||
*/
|
||||
private function isHidden(FieldDefinition $field): bool
|
||||
public static function isHiddenField(FieldDefinition $field): bool
|
||||
{
|
||||
if ($field->type === 'password') {
|
||||
return true;
|
||||
|
||||
@@ -98,6 +98,7 @@ final class FieldValidator
|
||||
switch ($field->type) {
|
||||
case 'string':
|
||||
case 'text':
|
||||
case 'password':
|
||||
return is_string($value) ? null : 'Text erwartet.';
|
||||
case 'integer':
|
||||
return is_int($value) ? null : 'Ganzzahl erwartet.';
|
||||
|
||||
@@ -9,6 +9,7 @@ use LIT\MCP\Abilities\Cursor;
|
||||
use LIT\MCP\Integrations\WordPress\Content\ContentQueryAbility;
|
||||
use LIT\MCP\Integrations\WordPress\Content\PostPresenter;
|
||||
use LIT\MCP\Integrations\WordPress\Schema\ContentSchema;
|
||||
use LIT\MCP\Integrations\WordPress\Schema\FieldDefinition;
|
||||
use LIT\MCP\Tests\Unit\TestCase;
|
||||
use Mockery;
|
||||
use WP_Post;
|
||||
@@ -28,7 +29,7 @@ final class ContentQueryAbilityTest extends TestCase
|
||||
Functions\when('current_user_can')->justReturn(true);
|
||||
Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) []]);
|
||||
$schema = Mockery::mock(ContentSchema::class);
|
||||
$schema->allows('fields')->andReturn(['level' => 'x']);
|
||||
$schema->allows('fields')->andReturn(['level' => new FieldDefinition('level', 'Level', 'string', 'pods', 'writable')]);
|
||||
$schema->allows('taxonomies')->andReturn([['name' => 'category', 'label' => 'Kategorien', 'hierarchical' => true]]);
|
||||
Functions\expect('get_posts')->once()->with(Mockery::on(static function (array $args): bool {
|
||||
return $args['post_type'] === 'lit_person'
|
||||
@@ -69,7 +70,7 @@ final class ContentQueryAbilityTest extends TestCase
|
||||
Functions\when('current_user_can')->justReturn(true);
|
||||
Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) []]);
|
||||
$schema = Mockery::mock(ContentSchema::class);
|
||||
$schema->allows('fields')->andReturn(['role' => 'x']);
|
||||
$schema->allows('fields')->andReturn(['role' => new FieldDefinition('role', 'Funktion', 'string', 'pods', 'writable')]);
|
||||
Functions\expect('get_posts')->once()->with(Mockery::on(
|
||||
static fn (array $args): bool => $args['meta_query'] === [['key' => 'role', 'compare' => 'EXISTS']]
|
||||
&& $args['post_status'] === ['publish', 'draft', 'pending', 'private', 'future']
|
||||
@@ -142,10 +143,53 @@ final class ContentQueryAbilityTest extends TestCase
|
||||
$this->ability()->execute(['post_type' => 'page']);
|
||||
}
|
||||
|
||||
public function testRequestingOnlyDisallowedStatusesReturnsEmptyWithoutQuerying(): void
|
||||
{
|
||||
Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) [
|
||||
'edit_others_posts' => 'edit_others_posts',
|
||||
'edit_published_posts' => 'edit_published_posts',
|
||||
]]);
|
||||
Functions\when('current_user_can')->alias(static fn (string $cap): bool => $cap === 'edit_others_posts');
|
||||
Functions\expect('get_posts')->never();
|
||||
|
||||
$result = $this->ability()->execute(['post_type' => 'page', 'status' => ['publish']]);
|
||||
|
||||
self::assertSame(['items' => [], 'next_cursor' => null], $result);
|
||||
}
|
||||
|
||||
public function testRequestingOnlyPrivateWithoutRightsReturnsEmptyWithoutQuerying(): void
|
||||
{
|
||||
Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) [
|
||||
'edit_others_posts' => 'edit_others_posts',
|
||||
'edit_private_posts' => 'edit_private_posts',
|
||||
]]);
|
||||
Functions\when('current_user_can')->alias(static fn (string $cap): bool => $cap === 'edit_others_posts');
|
||||
Functions\expect('get_posts')->never();
|
||||
|
||||
$result = $this->ability()->execute(['post_type' => 'page', 'status' => ['private']]);
|
||||
|
||||
self::assertSame(['items' => [], 'next_cursor' => null], $result);
|
||||
}
|
||||
|
||||
public function testOwnPrivatePostsStayVisibleWithoutEditPrivatePostsWhenAuthorRestricted(): void
|
||||
{
|
||||
Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) [
|
||||
'edit_others_posts' => 'edit_others_posts',
|
||||
'edit_private_posts' => 'edit_private_posts',
|
||||
]]);
|
||||
Functions\when('current_user_can')->justReturn(false);
|
||||
Functions\when('get_current_user_id')->justReturn(9);
|
||||
Functions\expect('get_posts')->once()->with(Mockery::on(
|
||||
static fn (array $args): bool => $args['post_status'] === ['private'] && ($args['author'] ?? null) === 9
|
||||
))->andReturn([]);
|
||||
|
||||
$this->ability()->execute(['post_type' => 'page', 'status' => ['private']]);
|
||||
}
|
||||
|
||||
public function testUnknownFieldKeyIsRejected(): void
|
||||
{
|
||||
$schema = Mockery::mock(ContentSchema::class);
|
||||
$schema->allows('fields')->andReturn(['role' => 'x']);
|
||||
$schema->allows('fields')->andReturn(['role' => new FieldDefinition('role', 'Funktion', 'string', 'pods', 'writable')]);
|
||||
|
||||
$result = $this->ability($schema)->execute(['post_type' => 'page', 'field' => ['key' => 'ghost']]);
|
||||
|
||||
@@ -153,6 +197,24 @@ final class ContentQueryAbilityTest extends TestCase
|
||||
self::assertStringContainsString('role', $result->get_error_message());
|
||||
}
|
||||
|
||||
public function testHiddenFieldKeyIsRejectedLikeAnUnknownOne(): void
|
||||
{
|
||||
$schema = Mockery::mock(ContentSchema::class);
|
||||
$schema->allows('fields')->andReturn([
|
||||
'role' => new FieldDefinition('role', 'Funktion', 'string', 'pods', 'writable'),
|
||||
'secret' => new FieldDefinition('secret', 'Secret', 'password', 'pods', 'writable'),
|
||||
'_internal' => new FieldDefinition('_internal', 'Internal', 'string', 'meta', 'readonly'),
|
||||
]);
|
||||
|
||||
$password = $this->ability($schema)->execute(['post_type' => 'page', 'field' => ['key' => 'secret']]);
|
||||
$protected = $this->ability($schema)->execute(['post_type' => 'page', 'field' => ['key' => '_internal']]);
|
||||
|
||||
self::assertSame('lit_validation_failed', $password->get_error_code());
|
||||
self::assertSame('Verfügbare Felder: role.', $password->get_error_data()['hint']);
|
||||
self::assertSame('lit_validation_failed', $protected->get_error_code());
|
||||
self::assertSame('Verfügbare Felder: role.', $protected->get_error_data()['hint']);
|
||||
}
|
||||
|
||||
public function testUnknownTaxonomyIsRejected(): void
|
||||
{
|
||||
$schema = Mockery::mock(ContentSchema::class);
|
||||
|
||||
@@ -31,6 +31,7 @@ final class FieldValidatorTest extends TestCase
|
||||
'start' => new FieldDefinition('start', 'Beginn', 'date', 'pods', 'writable'),
|
||||
'secret' => new FieldDefinition('_secret', '_secret', 'string', 'meta', 'readonly'),
|
||||
'city' => new FieldDefinition('city', 'city', 'unknown', 'sample', 'unknown'),
|
||||
'pin' => new FieldDefinition('pin', 'PIN', 'password', 'pods', 'writable'),
|
||||
];
|
||||
}
|
||||
|
||||
@@ -90,6 +91,14 @@ final class FieldValidatorTest extends TestCase
|
||||
self::assertSame('5 ist kein Medium der Mediathek.', $errors['photos']);
|
||||
}
|
||||
|
||||
public function testPasswordFieldsAreValidatedAsStrings(): void
|
||||
{
|
||||
$validator = new FieldValidator();
|
||||
|
||||
self::assertSame([], $validator->validate($this->definitions(), ['pin' => '1234'], false)['errors']);
|
||||
self::assertSame('Text erwartet.', $validator->validate($this->definitions(), ['pin' => 1234], false)['errors']['pin']);
|
||||
}
|
||||
|
||||
public function testUnknownTypedFieldsAreAcceptedWithAWarningAndNullClears(): void
|
||||
{
|
||||
$result = (new FieldValidator())->validate($this->definitions(), ['city' => ['any' => 'thing'], 'level' => null], false);
|
||||
|
||||
Reference in New Issue
Block a user