fix(security): headingLevel prüfen, überschreibende Tools kennzeichnen, Meta-Tools auf lit/ beschränken
- Divi-Vollformat: headingLevel nur h1 bis h6 (ValueCleaner, StoredNodeGuard, ShortformWriter); sonst ließ sich für Konten ohne unfiltered_html ein Skript-Element erzeugen. - content-update, divi-update und wp-settings-update sind destructive, damit MCP-Clients nachfragen. - mcp-adapter/execute-ability und get-ability-info nehmen auf dem OAuth-Server nur lit/-Abilities an. - release-publish.sh übergibt den Gitea-Token per Konfigurationsdatei statt als curl-Argument. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
194fd9e183
commit
165e61d7fe
@@ -7,6 +7,8 @@ namespace LIT\MCP\Tests\Unit;
|
||||
use Brain\Monkey\Filters;
|
||||
use Brain\Monkey\Functions;
|
||||
use LIT\MCP\Access;
|
||||
use LIT\MCP\Server\RequestContext;
|
||||
use WP_REST_Request;
|
||||
use WP_User;
|
||||
|
||||
final class AccessTest extends TestCase
|
||||
@@ -93,4 +95,73 @@ final class AccessTest extends TestCase
|
||||
|
||||
self::assertNotFalse(has_filter('user_has_cap', [$access, 'grantCapability']));
|
||||
}
|
||||
|
||||
protected function tearDown(): void
|
||||
{
|
||||
RequestContext::reset();
|
||||
parent::tearDown();
|
||||
}
|
||||
|
||||
/** @return array<string, mixed> The args of a meta tool as the adapter registers them (permission always granted). */
|
||||
private function metaToolArgs(): array
|
||||
{
|
||||
return ['permission_callback' => static fn (mixed $input = null): bool => true];
|
||||
}
|
||||
|
||||
private function asOAuthServerRequest(): void
|
||||
{
|
||||
RequestContext::recordRoute(null, [], new WP_REST_Request('/mcp/mcp-oauth-server'));
|
||||
}
|
||||
|
||||
public function testRegistersTheFilterThatLimitsTheAdapterMetaTools(): void
|
||||
{
|
||||
$access = new Access();
|
||||
$access->register();
|
||||
|
||||
self::assertNotFalse(has_filter('wp_register_ability_args', [$access, 'limitMetaToolsToOwnAbilities']));
|
||||
}
|
||||
|
||||
/** Security audit 2026-11, B3: execute-ability and get-ability-info may only name lit/ abilities. */
|
||||
public function testMetaToolsRefuseForeignAbilitiesOnTheOAuthServer(): void
|
||||
{
|
||||
$this->asOAuthServerRequest();
|
||||
|
||||
foreach (['mcp-adapter/execute-ability', 'mcp-adapter/get-ability-info'] as $meta) {
|
||||
$permission = (new Access())->limitMetaToolsToOwnAbilities($this->metaToolArgs(), $meta)['permission_callback'];
|
||||
|
||||
self::assertTrue($permission(['ability_name' => 'lit/guide']), $meta);
|
||||
foreach (['akismet/comment-check', 'elementor/delete-page', 'litx/guide', '', 'LIT/guide'] as $foreign) {
|
||||
$result = $permission(['ability_name' => $foreign]);
|
||||
|
||||
self::assertInstanceOf(\WP_Error::class, $result, $meta . ' ' . $foreign);
|
||||
self::assertSame('lit_forbidden', $result->get_error_code());
|
||||
}
|
||||
self::assertInstanceOf(\WP_Error::class, $permission(null), 'no ability_name');
|
||||
self::assertInstanceOf(\WP_Error::class, $permission(['ability_name' => ['lit/guide']]), 'not a string');
|
||||
}
|
||||
}
|
||||
|
||||
public function testMetaToolsKeepTheirOwnCheckOutsideTheOAuthServer(): void
|
||||
{
|
||||
$permission = (new Access())
|
||||
->limitMetaToolsToOwnAbilities($this->metaToolArgs(), 'mcp-adapter/execute-ability')['permission_callback'];
|
||||
|
||||
self::assertTrue($permission(['ability_name' => 'akismet/comment-check']));
|
||||
}
|
||||
|
||||
public function testOtherAbilitiesAreLeftAlone(): void
|
||||
{
|
||||
$args = $this->metaToolArgs();
|
||||
|
||||
self::assertSame($args, (new Access())->limitMetaToolsToOwnAbilities($args, 'lit/guide'));
|
||||
self::assertSame($args, (new Access())->limitMetaToolsToOwnAbilities($args, 'mcp-adapter/discover-abilities'));
|
||||
}
|
||||
|
||||
public function testAMetaToolWithoutAPermissionCallbackFailsClosed(): void
|
||||
{
|
||||
$this->asOAuthServerRequest();
|
||||
$permission = (new Access())->limitMetaToolsToOwnAbilities([], 'mcp-adapter/execute-ability')['permission_callback'];
|
||||
|
||||
self::assertFalse($permission(['ability_name' => 'lit/guide']));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -514,4 +514,20 @@ final class StoredNodeGuardTest extends TestCase
|
||||
self::assertSame([], self::guard(true, true, true)->problems($user));
|
||||
self::assertSame([], self::guard(true, true, true)->problems($private));
|
||||
}
|
||||
|
||||
/** Security audit 2026-11, B1: a copied heading keeps h1 to h6 only – "script" would become an element. */
|
||||
public function testACopiedHeadingTagMustBeH1ToH6(): void
|
||||
{
|
||||
$heading = static fn (string $level): Block => self::block(self::node('divi/heading', ['title' => [
|
||||
'decoration' => ['font' => ['font' => ['desktop' => ['value' => ['headingLevel' => $level]]]]],
|
||||
]]));
|
||||
|
||||
self::assertSame([], self::guard()->problems($heading('h2')));
|
||||
foreach (['script', 'H2', 'h9', 'h2><script>'] as $level) {
|
||||
$problems = self::guard()->problems($heading($level));
|
||||
|
||||
self::assertNotSame([], $problems, $level);
|
||||
self::assertStringContainsString('headingLevel', implode(' | ', $problems));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -350,4 +350,39 @@ final class ValueCleanerTest extends TestCase
|
||||
self::assertFalse($cleaner->keepsId("p\x01"), 'control characters would be stripped');
|
||||
self::assertFalse($cleaner->keepsId(ComposeFixture::REFERENCE_ALT), 'an id is never a $variable reference');
|
||||
}
|
||||
|
||||
/** Security audit 2026-11, B1: Divi turns headingLevel into an element name, so only h1 to h6 may be stored. */
|
||||
public function testOnlyHeadingTagsH1ToH6AreStoredAsHeadingLevel(): void
|
||||
{
|
||||
$cleaner = self::loopCleaner(false, false);
|
||||
$write = static fn (string $level): mixed => $cleaner->clean(
|
||||
'divi/heading',
|
||||
'title.decoration.font.font',
|
||||
['desktop' => ['value' => ['headingLevel' => $level]]]
|
||||
);
|
||||
|
||||
foreach (['h1', 'h2', 'h3', 'h4', 'h5', 'h6'] as $level) {
|
||||
self::assertSame(['desktop' => ['value' => ['headingLevel' => $level]]], $write($level));
|
||||
}
|
||||
foreach (['script', 'H1', 'h7', 'h0', 'h1 ', 'h1><script>', 'style', ''] as $level) {
|
||||
try {
|
||||
$write($level);
|
||||
self::fail('"' . $level . '" must be refused');
|
||||
} catch (InvalidArgumentException $e) {
|
||||
self::assertStringContainsString('h1 bis h6', $e->getMessage());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** The same key is checked at any depth, also without the desktop/value wrapper. */
|
||||
public function testHeadingLevelIsCheckedAtAnyDepth(): void
|
||||
{
|
||||
$this->expectException(InvalidArgumentException::class);
|
||||
|
||||
self::loopCleaner(false, false)->clean(
|
||||
'divi/heading',
|
||||
'title.decoration.font.font',
|
||||
['desktop' => ['headingLevel' => 'script']]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -308,4 +308,13 @@ final class DiviUpdateAbilityTest extends DiviWriteAbilityTestCase
|
||||
self::assertStringStartsWith('Keine Änderung', $result['summary']);
|
||||
self::assertSame([], $this->saved);
|
||||
}
|
||||
|
||||
/** Security audit 2026-11, B2: changing attributes of existing modules overwrites them. */
|
||||
public function testOverwritesSoItIsMarkedDestructive(): void
|
||||
{
|
||||
$annotations = $this->update()->annotations();
|
||||
|
||||
self::assertTrue($annotations['destructive']);
|
||||
self::assertFalse($annotations['readonly']);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -374,4 +374,13 @@ final class ContentUpdateAbilityTest extends TestCase
|
||||
unlink($file);
|
||||
$log->assertRecorded([]);
|
||||
}
|
||||
|
||||
/** Security audit 2026-11, B2: overwriting a post must not look like "only adds" to an MCP client. */
|
||||
public function testOverwritesSoItIsMarkedDestructive(): void
|
||||
{
|
||||
$annotations = $this->ability(\Mockery::mock(PostWriter::class))->annotations();
|
||||
|
||||
self::assertTrue($annotations['destructive']);
|
||||
self::assertFalse($annotations['readonly']);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -284,4 +284,13 @@ final class WpSettingsAbilitiesTest extends TestCase
|
||||
(new WpSettingsUpdateAbility(new SettingsWhitelist(), FakeChangeLog::forbidden()))->permission([])->get_error_code()
|
||||
);
|
||||
}
|
||||
|
||||
/** Security audit 2026-11, B2: site settings (front page, permalinks, title) are overwritten. */
|
||||
public function testUpdatingSettingsIsMarkedDestructive(): void
|
||||
{
|
||||
$annotations = (new WpSettingsUpdateAbility(new SettingsWhitelist(), FakeChangeLog::forbidden()))->annotations();
|
||||
|
||||
self::assertTrue($annotations['destructive']);
|
||||
self::assertFalse($annotations['readonly']);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user