From 0890e6f165ebb3b24f786e3940cfe7d5551f82d3 Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Fri, 25 Sep 2026 22:48:05 +0200 Subject: [PATCH] fix: restrict content-query by cap before pagination, filter hidden fields Review fix round 1: content-query now excludes posts/statuses the user cannot edit before querying (author restriction without edit_others_posts, status filtering by edit_private_posts/edit_published_posts) instead of only dropping them after fetching, and rejects unknown field/taxonomy filters. content-types' not-found hint only lists editable types. PostPresenter skips Pods password fields and readonly protected meta. Query input gains search/terms length limits. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7 --- src/Integrations/Pods/PodsFieldSource.php | 2 +- .../WordPress/Content/ContentQueryAbility.php | 100 ++++++++++++++++- .../WordPress/Content/ContentTypesAbility.php | 11 +- .../WordPress/Content/PostPresenter.php | 20 ++++ .../Content/ContentQueryAbilityTest.php | 104 +++++++++++++++++- .../Content/ContentTypesAbilityTest.php | 19 ++++ .../WordPress/Content/PostPresenterTest.php | 22 ++++ 7 files changed, 268 insertions(+), 10 deletions(-) diff --git a/src/Integrations/Pods/PodsFieldSource.php b/src/Integrations/Pods/PodsFieldSource.php index 2cc0b1c..fc22685 100644 --- a/src/Integrations/Pods/PodsFieldSource.php +++ b/src/Integrations/Pods/PodsFieldSource.php @@ -18,7 +18,7 @@ final class PodsFieldSource implements FieldSource { private const TYPES = [ 'text' => 'string', 'website' => 'string', 'phone' => 'string', 'email' => 'string', - 'password' => 'string', 'slug' => 'string', 'color' => 'string', 'oembed' => 'string', + 'password' => 'password', 'slug' => 'string', 'color' => 'string', 'oembed' => 'string', 'paragraph' => 'text', 'wysiwyg' => 'text', 'code' => 'text', 'number' => 'number', 'currency' => 'number', 'boolean' => 'boolean', diff --git a/src/Integrations/WordPress/Content/ContentQueryAbility.php b/src/Integrations/WordPress/Content/ContentQueryAbility.php index 48793d8..898a8c0 100644 --- a/src/Integrations/WordPress/Content/ContentQueryAbility.php +++ b/src/Integrations/WordPress/Content/ContentQueryAbility.php @@ -6,6 +6,7 @@ namespace LIT\MCP\Integrations\WordPress\Content; use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Cursor; +use LIT\MCP\Abilities\Errors; use LIT\MCP\Integrations\WordPress\PostGuard; use LIT\MCP\Integrations\WordPress\Schema\ContentSchema; use WP_Error; @@ -48,12 +49,17 @@ final class ContentQueryAbility implements Ability 'items' => ['type' => 'string', 'enum' => self::STATUSES], 'description' => 'Standard: alle außer Papierkorb.', ], - 'search' => ['type' => 'string'], + 'search' => ['type' => 'string', 'maxLength' => 200], 'taxonomy' => [ 'type' => 'object', 'properties' => [ 'taxonomy' => ['type' => 'string'], - 'terms' => ['type' => 'array', 'minItems' => 1, 'items' => ['type' => ['integer', 'string']]], + 'terms' => [ + 'type' => 'array', + 'minItems' => 1, + 'maxItems' => 50, + 'items' => ['type' => ['integer', 'string']], + ], ], 'required' => ['taxonomy', 'terms'], 'additionalProperties' => false, @@ -115,6 +121,22 @@ final class ContentQueryAbility implements Ability return Cursor::invalid(); } + $postType = (string) $input['post_type']; + + if (isset($input['field']) && is_array($input['field'])) { + $error = $this->rejectUnlessKnownField($postType, (string) $input['field']['key']); + if ($error !== null) { + return $error; + } + } + + if (isset($input['taxonomy']) && is_array($input['taxonomy'])) { + $error = $this->rejectUnlessKnownTaxonomy($postType, (string) $input['taxonomy']['taxonomy']); + if ($error !== null) { + return $error; + } + } + $limit = Cursor::limit($input['limit'] ?? null); $posts = get_posts(array_merge($this->queryArgs($input), [ 'posts_per_page' => $limit + 1, @@ -138,9 +160,13 @@ final class ContentQueryAbility implements Ability */ private function queryArgs(array $input): array { + $postType = (string) $input['post_type']; + $typeObject = get_post_type_object($postType); + $caps = is_object($typeObject) && isset($typeObject->cap) ? $typeObject->cap : null; + $args = [ - 'post_type' => (string) $input['post_type'], - 'post_status' => $input['status'] ?? self::STATUSES, + 'post_type' => $postType, + 'post_status' => $this->allowedStatuses((array) ($input['status'] ?? self::STATUSES), $caps), 'orderby' => (string) ($input['orderby'] ?? 'date'), 'order' => strtoupper((string) ($input['order'] ?? 'desc')), 'no_found_rows' => true, @@ -148,6 +174,13 @@ 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'))) { + $args['author'] = get_current_user_id(); + } + if (isset($input['search']) && $input['search'] !== '') { $args['s'] = (string) $input['search']; } @@ -204,4 +237,63 @@ final class ContentQueryAbility implements Ability return count($clauses) > 1 ? array_merge(['relation' => 'OR'], $clauses) : $clauses; } + + /** + * Drops statuses the current user has no right to edit at all, so the query never + * 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). + * + * @param list $statuses + * @return list + */ + private function allowedStatuses(array $statuses, ?object $caps): array + { + $gates = [ + 'private' => $this->capability($caps, 'edit_private_posts'), + 'publish' => $this->capability($caps, 'edit_published_posts'), + 'future' => $this->capability($caps, 'edit_published_posts'), + ]; + + return array_values(array_filter( + array_map('strval', $statuses), + static fn (string $status): bool => ! isset($gates[$status]) || current_user_can($gates[$status]) + )); + } + + private function capability(?object $caps, string $name): string + { + return $caps !== null && isset($caps->{$name}) ? (string) $caps->{$name} : $name; + } + + private function rejectUnlessKnownField(string $postType, string $key): ?WP_Error + { + $allowed = array_keys($this->schema->fields($postType)); + if (in_array($key, $allowed, true)) { + return null; + } + + return Errors::invalidInput( + sprintf('Feld "%s" ist für Inhaltstyp "%s" nicht verfügbar.', $key, $postType), + ['field.key' => 'unbekanntes Feld'], + $allowed === [] + ? 'Für diesen Inhaltstyp sind keine Felder verfügbar.' + : 'Verfügbare Felder: ' . implode(', ', $allowed) . '.' + ); + } + + private function rejectUnlessKnownTaxonomy(string $postType, string $taxonomy): ?WP_Error + { + $allowed = array_column($this->schema->taxonomies($postType), 'name'); + if (in_array($taxonomy, $allowed, true)) { + return null; + } + + return Errors::invalidInput( + sprintf('Taxonomie "%s" ist für Inhaltstyp "%s" nicht verfügbar.', $taxonomy, $postType), + ['taxonomy.taxonomy' => 'unbekannte Taxonomie'], + $allowed === [] + ? 'Für diesen Inhaltstyp sind keine Taxonomien verfügbar.' + : 'Verfügbare Taxonomien: ' . implode(', ', $allowed) . '.' + ); + } } diff --git a/src/Integrations/WordPress/Content/ContentTypesAbility.php b/src/Integrations/WordPress/Content/ContentTypesAbility.php index f8f9c69..aac7b32 100644 --- a/src/Integrations/WordPress/Content/ContentTypesAbility.php +++ b/src/Integrations/WordPress/Content/ContentTypesAbility.php @@ -99,13 +99,22 @@ final class ContentTypesAbility implements Ability if ($description === null) { return Errors::notFound( sprintf('Inhaltstyp "%s" ist nicht verfügbar.', $postType), - 'Verfügbare Inhaltstypen: ' . implode(', ', $this->schema->postTypeNames()) . '.' + 'Verfügbare Inhaltstypen: ' . implode(', ', $this->editableTypeNames()) . '.' ); } return ['post_types' => [$description]]; } + /** @return list */ + private function editableTypeNames(): array + { + return array_values(array_filter( + $this->schema->postTypeNames(), + fn (string $name): bool => $this->canEdit($name) + )); + } + /** * Only post types the user can edit are listed or described (spec §4.5): requesting * a managed but non-editable type is a forbidden error, not a silent omission. diff --git a/src/Integrations/WordPress/Content/PostPresenter.php b/src/Integrations/WordPress/Content/PostPresenter.php index 44cf1c3..939bb1c 100644 --- a/src/Integrations/WordPress/Content/PostPresenter.php +++ b/src/Integrations/WordPress/Content/PostPresenter.php @@ -7,6 +7,7 @@ namespace LIT\MCP\Integrations\WordPress\Content; use LIT\MCP\Abilities\JsonMap; use LIT\MCP\Abilities\Rev; use LIT\MCP\Integrations\WordPress\Schema\ContentSchema; +use LIT\MCP\Integrations\WordPress\Schema\FieldDefinition; use WP_Post; /** @@ -143,12 +144,31 @@ class PostPresenter { $result = []; foreach ($this->schema->fields((string) $post->post_type) as $key => $field) { + if ($this->isHidden($field)) { + continue; + } + $result[$key] = $this->schema->sourceOf($field)->read((int) $post->ID, $field); } return $result; } + /** + * 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. + */ + private function isHidden(FieldDefinition $field): bool + { + if ($field->type === 'password') { + return true; + } + + return $field->status !== FieldDefinition::WRITABLE && str_starts_with($field->key, '_'); + } + /** @return array{id: int, url: string, alt: string}|null */ private function featuredImage(WP_Post $post): ?array { diff --git a/tests/Unit/Integrations/WordPress/Content/ContentQueryAbilityTest.php b/tests/Unit/Integrations/WordPress/Content/ContentQueryAbilityTest.php index 8caf6c5..c073e20 100644 --- a/tests/Unit/Integrations/WordPress/Content/ContentQueryAbilityTest.php +++ b/tests/Unit/Integrations/WordPress/Content/ContentQueryAbilityTest.php @@ -15,17 +15,21 @@ use WP_Post; final class ContentQueryAbilityTest extends TestCase { - private function ability(): ContentQueryAbility + private function ability(?ContentSchema $schema = null): ContentQueryAbility { $presenter = Mockery::mock(PostPresenter::class); $presenter->allows('summary')->andReturnUsing(static fn (WP_Post $post): array => ['id' => $post->ID]); - return new ContentQueryAbility(Mockery::mock(ContentSchema::class), $presenter); + return new ContentQueryAbility($schema ?? Mockery::mock(ContentSchema::class), $presenter); } public function testBuildsTheQueryAndPaginates(): void { 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('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' && $args['post_status'] === ['draft'] @@ -43,7 +47,7 @@ final class ContentQueryAbilityTest extends TestCase && $args['date_query'] === [['after' => '2026-01-01', 'inclusive' => true]]; }))->andReturn([new WP_Post(['ID' => 1]), new WP_Post(['ID' => 2]), new WP_Post(['ID' => 3])]); - $result = $this->ability()->execute([ + $result = $this->ability($schema)->execute([ 'post_type' => 'lit_person', 'status' => ['draft'], 'search' => 'Anna', @@ -63,12 +67,15 @@ final class ContentQueryAbilityTest extends TestCase public function testExistsFilterHasNoValueAndLastPageHasNoCursor(): void { 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']); 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'] ))->andReturn([new WP_Post(['ID' => 1])]); - $result = $this->ability()->execute(['post_type' => 'page', 'field' => ['key' => 'role', 'compare' => 'EXISTS']]); + $result = $this->ability($schema)->execute(['post_type' => 'page', 'field' => ['key' => 'role', 'compare' => 'EXISTS']]); self::assertNull($result['next_cursor']); } @@ -76,6 +83,7 @@ final class ContentQueryAbilityTest extends TestCase public function testPostsTheUserCannotEditAreDropped(): void { Functions\when('current_user_can')->alias(static fn (string $cap, int $id = 0): bool => $id !== 2); + Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) []]); Functions\when('get_posts')->justReturn([new WP_Post(['ID' => 1]), new WP_Post(['ID' => 2])]); $result = $this->ability()->execute(['post_type' => 'page']); @@ -90,6 +98,75 @@ final class ContentQueryAbilityTest extends TestCase self::assertSame('lit_validation_failed', $result->get_error_code()); } + public function testQueryIsRestrictedToOwnPostsWithoutEditOthersPosts(): void + { + Functions\when('get_post_type_object')->justReturn( + (object) ['cap' => (object) ['edit_others_posts' => 'edit_others_posts']] + ); + Functions\when('current_user_can')->alias(static fn (string $cap): bool => $cap !== 'edit_others_posts'); + Functions\when('get_current_user_id')->justReturn(7); + Functions\expect('get_posts')->once()->with( + Mockery::on(static fn (array $args): bool => ($args['author'] ?? null) === 7) + )->andReturn([]); + + $result = $this->ability()->execute(['post_type' => 'page']); + + self::assertSame([], $result['items']); + } + + public function testQueryIsNotRestrictedToAuthorWithEditOthersPosts(): void + { + Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) []]); + Functions\when('current_user_can')->justReturn(true); + Functions\expect('get_posts')->once()->with( + Mockery::on(static fn (array $args): bool => ! array_key_exists('author', $args)) + )->andReturn([]); + + $this->ability()->execute(['post_type' => 'page']); + } + + public function testDisallowedStatusesAreRemovedFromTheQuery(): void + { + Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) [ + 'edit_others_posts' => 'edit_others_posts', + 'edit_private_posts' => 'edit_private_posts', + 'edit_published_posts' => 'edit_published_posts', + ]]); + Functions\when('current_user_can')->alias( + static fn (string $cap): bool => ! in_array($cap, ['edit_private_posts', 'edit_published_posts'], true) + ); + Functions\expect('get_posts')->once()->with( + Mockery::on(static fn (array $args): bool => $args['post_status'] === ['draft', 'pending']) + )->andReturn([]); + + $this->ability()->execute(['post_type' => 'page']); + } + + public function testUnknownFieldKeyIsRejected(): void + { + $schema = Mockery::mock(ContentSchema::class); + $schema->allows('fields')->andReturn(['role' => 'x']); + + $result = $this->ability($schema)->execute(['post_type' => 'page', 'field' => ['key' => 'ghost']]); + + self::assertSame('lit_validation_failed', $result->get_error_code()); + self::assertStringContainsString('role', $result->get_error_message()); + } + + public function testUnknownTaxonomyIsRejected(): void + { + $schema = Mockery::mock(ContentSchema::class); + $schema->allows('taxonomies')->andReturn([['name' => 'category', 'label' => 'Kategorien', 'hierarchical' => true]]); + + $result = $this->ability($schema)->execute([ + 'post_type' => 'page', + 'taxonomy' => ['taxonomy' => 'ghost', 'terms' => [1]], + ]); + + self::assertSame('lit_validation_failed', $result->get_error_code()); + self::assertStringContainsString('category', $result->get_error_message()); + } + public function testPermissionNeedsAManagedTypeAndEditRights(): void { $schema = Mockery::mock(ContentSchema::class); @@ -98,4 +175,23 @@ final class ContentQueryAbilityTest extends TestCase self::assertSame('lit_not_found', $ability->permission(['post_type' => 'secret'])->get_error_code()); } + + public function testPermissionIsForbiddenWithoutEditRights(): void + { + Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) ['edit_posts' => 'edit_posts']]); + Functions\when('current_user_can')->justReturn(false); + $schema = Mockery::mock(ContentSchema::class); + $schema->allows('isManaged')->andReturn(true); + $ability = new ContentQueryAbility($schema, Mockery::mock(PostPresenter::class)); + + self::assertSame('lit_forbidden', $ability->permission(['post_type' => 'page'])->get_error_code()); + } + + public function testInputSchemaLimitsSearchLengthAndTaxonomyTermsCount(): void + { + $schema = $this->ability()->inputSchema(); + + self::assertSame(200, $schema['properties']['search']['maxLength']); + self::assertSame(50, $schema['properties']['taxonomy']['properties']['terms']['maxItems']); + } } diff --git a/tests/Unit/Integrations/WordPress/Content/ContentTypesAbilityTest.php b/tests/Unit/Integrations/WordPress/Content/ContentTypesAbilityTest.php index 889ee5f..6edf341 100644 --- a/tests/Unit/Integrations/WordPress/Content/ContentTypesAbilityTest.php +++ b/tests/Unit/Integrations/WordPress/Content/ContentTypesAbilityTest.php @@ -54,6 +54,8 @@ final class ContentTypesAbilityTest extends TestCase public function testUnknownTypeIsNotFoundWithTheAvailableTypes(): void { + Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) ['edit_posts' => 'edit_posts']]); + Functions\when('current_user_can')->justReturn(true); $schema = Mockery::mock(ContentSchema::class); $schema->allows('isManaged')->andReturn(false); $schema->allows('describe')->andReturn(null); @@ -66,6 +68,23 @@ final class ContentTypesAbilityTest extends TestCase self::assertStringContainsString('page, post', $result->get_error_message()); } + public function testUnknownTypeHintOnlyListsTypesTheUserCanEdit(): void + { + Functions\when('get_post_type_object')->alias(static fn (string $type) => (object) [ + 'cap' => (object) ['edit_posts' => $type === 'page' ? 'edit_pages' : 'edit_posts'], + ]); + Functions\when('current_user_can')->alias(static fn (string $cap): bool => $cap === 'edit_posts'); + $schema = Mockery::mock(ContentSchema::class); + $schema->allows('isManaged')->andReturn(false); + $schema->allows('describe')->andReturn(null); + $schema->allows('postTypeNames')->andReturn(['page', 'post']); + + $result = (new ContentTypesAbility($schema))->execute(['post_type' => 'nope']); + + self::assertStringContainsString('post', $result->get_error_message()); + self::assertStringNotContainsString('Verfügbare Inhaltstypen: page', $result->get_error_message()); + } + public function testRequestingANonEditableSpecificTypeIsForbidden(): void { Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) ['edit_posts' => 'edit_pages']]); diff --git a/tests/Unit/Integrations/WordPress/Content/PostPresenterTest.php b/tests/Unit/Integrations/WordPress/Content/PostPresenterTest.php index e9394f8..a3055e1 100644 --- a/tests/Unit/Integrations/WordPress/Content/PostPresenterTest.php +++ b/tests/Unit/Integrations/WordPress/Content/PostPresenterTest.php @@ -90,6 +90,28 @@ final class PostPresenterTest extends TestCase self::assertStringContainsString('lit-divi-outline', $full['hints'][0]); self::assertInstanceOf(stdClass::class, $full['terms']); self::assertInstanceOf(stdClass::class, $full['fields']); + self::assertSame(Rev::of(''), $full['rev']); + } + + public function testPasswordAndProtectedReadonlyMetaFieldsAreOmitted(): void + { + $this->stubLinks(); + Functions\when('get_post_meta')->justReturn(''); + Functions\when('get_post_thumbnail_id')->justReturn(0); + $password = new FieldDefinition('secret', 'Secret', 'password', 'pods', 'writable'); + $protected = new FieldDefinition('_internal', 'Internal', 'string', 'meta', 'readonly'); + $visible = new FieldDefinition('role', 'Funktion', 'string', 'pods', 'writable'); + $source = Mockery::mock(FieldSource::class); + $source->expects('read')->once()->with(5, $visible)->andReturn('Leitung'); + $schema = Mockery::mock(ContentSchema::class); + $schema->allows('taxonomies')->andReturn([]); + $schema->allows('fields')->andReturn(['secret' => $password, '_internal' => $protected, 'role' => $visible]); + $schema->allows('sourceOf')->andReturn($source); + $post = new WP_Post(['ID' => 5, 'post_type' => 'lit_person']); + + $full = (new PostPresenter($schema))->full($post); + + self::assertSame(['role' => 'Leitung'], $full['fields']); } public function testDiviIsAlsoDetectedFromBlockMarkup(): void