fix: Preset-Änderungen ohne unfiltered_html ablehnen, wenn Divi HTML/CSS anderer Presets entfernen würde

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Jan-Niclas Loosen
2026-10-16 18:09:31 +02:00
co-authored by Claude Opus 5.5
parent 048cd9353d
commit 75714429be
10 changed files with 289 additions and 14 deletions
+33 -1
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace LIT\MCP\Integrations\Divi\Design; namespace LIT\MCP\Integrations\Divi\Design;
use Closure;
use LIT\MCP\Abilities\Errors; use LIT\MCP\Abilities\Errors;
use WP_Error; use WP_Error;
@@ -11,10 +12,41 @@ defined('ABSPATH') || exit;
/** /**
* Who may change the design system (spec §10): presets need edit_theme_options and Divi's role right "Visual Builder * Who may change the design system (spec §10): presets need edit_theme_options and Divi's role right "Visual Builder
* verwenden" – what Divi's own preset sync asks since 5.15 (formats "Nachtrag 5.15.1"). * verwenden" – what Divi's own preset sync asks since 5.15 (formats "Nachtrag 5.15.1"). Without unfiltered_html a
* write is refused while presets it saves along hold HTML/CSS that Divi would strip (final review I-2).
*/ */
final class DesignRights final class DesignRights
{ {
public const OTHER_MARKUP = 'Andere Presets enthalten HTML/CSS, das beim Speichern ohne das Recht unfiltered_html '
. 'verloren ginge; nichts wurde gespeichert.';
/** @return Closure(): bool Whether the current user may save unfiltered HTML. */
public static function unfilteredHtml(): Closure
{
return static fn (): bool => current_user_can('unfiltered_html');
}
/**
* Refuses the write when the user lacks unfiltered_html and $markupIds (PresetBook::markupOutside()) is not empty.
*
* @param Closure(): bool $unfiltered
* @param list<string> $markupIds
*/
public static function markup(
Closure $unfiltered,
array $markupIds,
string $message = self::OTHER_MARKUP
): ?WP_Error {
if ($markupIds === [] || $unfiltered()) {
return null;
}
return Errors::forbidden(
$message,
'Einen Administrator (mit dem Recht unfiltered_html) die Änderung vornehmen lassen.'
);
}
public static function presets(DesignSystemWriter $writer): ?WP_Error public static function presets(DesignSystemWriter $writer): ?WP_Error
{ {
if (! current_user_can('edit_theme_options')) { if (! current_user_can('edit_theme_options')) {
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace LIT\MCP\Integrations\Divi\Design; namespace LIT\MCP\Integrations\Divi\Design;
use Closure;
use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Ability;
use LIT\MCP\Abilities\Annotations; use LIT\MCP\Abilities\Annotations;
use LIT\MCP\Abilities\Errors; use LIT\MCP\Abilities\Errors;
@@ -20,11 +21,17 @@ final class DiviDesignRestoreAbility implements Ability
{ {
private const ID = '/^(colors|variables|presets|theme_options)-\d{8}T\d{6}Z-[0-9a-f]{6}$/D'; private const ID = '/^(colors|variables|presets|theme_options)-\d{8}T\d{6}Z-[0-9a-f]{6}$/D';
private const MARKUP = 'Der Snapshot oder der aktuelle Bestand enthält Presets mit HTML/CSS, das beim Speichern '
. 'ohne das Recht unfiltered_html verloren ginge; nichts wurde gespeichert.';
/** @param (Closure(): bool)|null $unfiltered Whether the user may save unfiltered HTML (DesignRights). */
public function __construct( public function __construct(
private DesignSystemWriter $writer, private DesignSystemWriter $writer,
private DesignChange $change, private DesignChange $change,
private SnapshotStore $snapshots private SnapshotStore $snapshots,
private ?Closure $unfiltered = null
) { ) {
$this->unfiltered ??= DesignRights::unfilteredHtml();
} }
public function name(): string public function name(): string
@@ -101,13 +108,23 @@ final class DiviDesignRestoreAbility implements Ability
); );
} }
$unfiltered = $this->unfiltered;
return $this->change->run( return $this->change->run(
$this->name(), $this->name(),
new PresetArea($this->writer), new PresetArea($this->writer),
$input, $input,
static function (array $raw) use ($snapshot, $id): DesignPlan { static function (array $raw) use ($snapshot, $id, $unfiltered): DesignPlan|WP_Error {
$before = PresetBook::fromDivi($raw); $before = PresetBook::fromDivi($raw);
$after = PresetBook::fromDivi($snapshot['data']); $after = PresetBook::fromDivi($snapshot['data']);
$refusal = DesignRights::markup(
$unfiltered,
array_merge($before->markupOutside([]), $after->markupOutside([])),
self::MARKUP
);
if ($refusal !== null) {
return $refusal;
}
$diff = PresetDiff::between($before, $after); $diff = PresetDiff::between($before, $after);
return new DesignPlan( return new DesignPlan(
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace LIT\MCP\Integrations\Divi\Design; namespace LIT\MCP\Integrations\Divi\Design;
use Closure;
use InvalidArgumentException; use InvalidArgumentException;
use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Ability;
use LIT\MCP\Abilities\Annotations; use LIT\MCP\Abilities\Annotations;
@@ -26,11 +27,14 @@ final class DiviPresetsDeleteAbility implements Ability
private const DETACH_HINT = 'Erst die Verweise entfernen: lit/divi-design-detach (ab Etappe 4b) oder im ' private const DETACH_HINT = 'Erst die Verweise entfernen: lit/divi-design-detach (ab Etappe 4b) oder im '
. 'Divi-Builder; danach erneut löschen.'; . 'Divi-Builder; danach erneut löschen.';
/** @param (Closure(): bool)|null $unfiltered Whether the user may save unfiltered HTML (DesignRights). */
public function __construct( public function __construct(
private DesignSystemWriter $writer, private DesignSystemWriter $writer,
private DesignChange $change, private DesignChange $change,
private GlobalUsages $usages private GlobalUsages $usages,
private ?Closure $unfiltered = null
) { ) {
$this->unfiltered ??= DesignRights::unfilteredHtml();
} }
public function name(): string public function name(): string
@@ -162,6 +166,10 @@ final class DiviPresetsDeleteAbility implements Ability
self::DETACH_HINT self::DETACH_HINT
); );
} }
$refusal = DesignRights::markup($this->unfiltered, $book->markupOutside($deleted));
if ($refusal !== null) {
return $refusal;
}
$diff = PresetDiff::between($before, $book); $diff = PresetDiff::between($before, $book);
@@ -30,17 +30,25 @@ final class DiviPresetsSetAbility implements Ability
private const NAME_RULE = 'Name: 1–100 Zeichen, ohne HTML'; private const NAME_RULE = 'Name: 1–100 Zeichen, ohne HTML';
private const MARKUP = 'Vorhandene Presets enthalten HTML/CSS, das beim Speichern ohne das Recht unfiltered_html '
. 'verloren ginge; nichts wurde gespeichert.';
private const GROUP_DEFAULT_WARNING = 'Standard-Gruppen-Presets wirken zusätzlich auf alle Module mit dieser ' private const GROUP_DEFAULT_WARNING = 'Standard-Gruppen-Presets wirken zusätzlich auf alle Module mit dieser '
. 'Gruppe ohne eigenes Gruppen-Preset; diese Seiten sind nicht aufgelistet.'; . 'Gruppe ohne eigenes Gruppen-Preset; diese Seiten sind nicht aufgelistet.';
/** @param Closure(): PresetComposer $composer */ /**
* @param Closure(): PresetComposer $composer
* @param (Closure(): bool)|null $unfiltered Whether the user may save unfiltered HTML (DesignRights).
*/
public function __construct( public function __construct(
private DesignSystemWriter $writer, private DesignSystemWriter $writer,
private DesignChange $change, private DesignChange $change,
private GlobalUsages $usages, private GlobalUsages $usages,
private DiviPageLoader $loader, private DiviPageLoader $loader,
private Closure $composer private Closure $composer,
private ?Closure $unfiltered = null
) { ) {
$this->unfiltered ??= DesignRights::unfilteredHtml();
} }
public function name(): string public function name(): string
@@ -150,6 +158,7 @@ final class DiviPresetsSetAbility implements Ability
$errors = []; $errors = [];
$invalid = []; $invalid = [];
$targets = []; $targets = [];
$created = [];
$warnings = []; $warnings = [];
foreach ($items as $index => $item) { foreach ($items as $index => $item) {
$field = sprintf('items[%d]', $index); $field = sprintf('items[%d]', $index);
@@ -169,6 +178,9 @@ final class DiviPresetsSetAbility implements Ability
continue; continue;
} }
$id = (string) $result['item']['id']; $id = (string) $result['item']['id'];
if ($existing === null) {
$created[] = $id;
}
$book = $book->with($scope, $result['item']); $book = $book->with($scope, $result['item']);
if (($item['make_default'] ?? null) === true) { if (($item['make_default'] ?? null) === true) {
$book = $book->withDefault($scope, $id); $book = $book->withDefault($scope, $id);
@@ -190,6 +202,11 @@ final class DiviPresetsSetAbility implements Ability
if ($errors !== []) { if ($errors !== []) {
return Errors::invalidInput('Ungültige Preset-Angaben; nichts wurde gespeichert.', $errors); return Errors::invalidInput('Ungültige Preset-Angaben; nichts wurde gespeichert.', $errors);
} }
// Only new presets passed the choke point as a whole; a changed one keeps its stored settings (review I-2).
$refusal = DesignRights::markup($this->unfiltered, $book->markupOutside($created), self::MARKUP);
if ($refusal !== null) {
return $refusal;
}
$diff = PresetDiff::between($before, $book); $diff = PresetDiff::between($before, $book);
@@ -200,6 +200,35 @@ final class PresetBook
return $users; return $users;
} }
/**
* IDs of the presets outside $touchedIds whose settings (attrs, styleAttrs, renderAttrs) hold a string with "<"
* or ">" – HTML or CSS that Divi's prepare_data() strips when a user without unfiltered_html saves the inventory
* (final review I-2).
*
* @param list<string> $touchedIds
* @return list<string>
*/
public function markupOutside(array $touchedIds): array
{
$ids = [];
foreach ($this->data as $records) {
foreach ($records as $record) {
foreach ($record['items'] as $id => $item) {
$id = (string) $id;
if (in_array($id, $touchedIds, true)) {
continue;
}
$settings = array_intersect_key($item, ['attrs' => 1, 'styleAttrs' => 1, 'renderAttrs' => 1]);
if (self::hasMarkup($settings)) {
$ids[] = $id;
}
}
}
}
return $ids;
}
/** @return array{module: list<array{default: string, items: list<array<string, mixed>>}>, /** @return array{module: list<array{default: string, items: list<array<string, mixed>>}>,
* group: list<array{default: string, items: list<array<string, mixed>>}>} */ * group: list<array{default: string, items: list<array<string, mixed>>}>} */
public function toRest(): array public function toRest(): array
@@ -225,6 +254,22 @@ final class PresetBook
return $id; return $id;
} }
private static function hasMarkup(mixed $value): bool
{
if (is_string($value)) {
return strpbrk($value, '<>') !== false;
}
if (is_array($value)) {
foreach ($value as $inner) {
if (self::hasMarkup($inner)) {
return true;
}
}
}
return false;
}
private static function plain(mixed $value): mixed private static function plain(mixed $value): mixed
{ {
if (is_object($value)) { if (is_object($value)) {
+15 -3
View File
@@ -11,6 +11,7 @@ use LIT\MCP\Integrations\Divi\Design\DesignSystemReader;
use LIT\MCP\Integrations\Divi\Design\DesignSystemWriter; use LIT\MCP\Integrations\Divi\Design\DesignSystemWriter;
use LIT\MCP\Integrations\Divi\Design\DiviDesignSystemAbility; use LIT\MCP\Integrations\Divi\Design\DiviDesignSystemAbility;
use LIT\MCP\Integrations\Divi\Design\DesignChange; use LIT\MCP\Integrations\Divi\Design\DesignChange;
use LIT\MCP\Integrations\Divi\Design\DesignRights;
use LIT\MCP\Integrations\Divi\Design\DiviPresetGetAbility; use LIT\MCP\Integrations\Divi\Design\DiviPresetGetAbility;
use LIT\MCP\Integrations\Divi\Design\DiviDesignRestoreAbility; use LIT\MCP\Integrations\Divi\Design\DiviDesignRestoreAbility;
use LIT\MCP\Integrations\Divi\Design\DiviDesignSnapshotsAbility; use LIT\MCP\Integrations\Divi\Design\DiviDesignSnapshotsAbility;
@@ -167,11 +168,22 @@ final class DiviIntegration implements Integration
$this->designChange(), $this->designChange(),
$this->globalUsages(), $this->globalUsages(),
$loader, $loader,
$this->presetComposer(...) $this->presetComposer(...),
DesignRights::unfilteredHtml()
),
new DiviPresetsDeleteAbility(
$this->designWriter,
$this->designChange(),
$this->globalUsages(),
DesignRights::unfilteredHtml()
), ),
new DiviPresetsDeleteAbility($this->designWriter, $this->designChange(), $this->globalUsages()),
new DiviDesignSnapshotsAbility(new SnapshotStore()), new DiviDesignSnapshotsAbility(new SnapshotStore()),
new DiviDesignRestoreAbility($this->designWriter, $this->designChange(), new SnapshotStore()), new DiviDesignRestoreAbility(
$this->designWriter,
$this->designChange(),
new SnapshotStore(),
DesignRights::unfilteredHtml()
),
]; ];
} }
@@ -46,6 +46,63 @@ final class DiviDesignRestoreAbilityTest extends TestCase
self::assertSame('presets-3', $result['snapshot_id']); self::assertSame('presets-3', $result['snapshot_id']);
} }
private static function snapshotOf(array $data): array
{
return ['id' => 'presets-20261016T100000Z-aaaaaa', 'data' => $data, 'created' => 'x', 'user_id' => 1,
'ability' => 'lit/divi-presets-set', 'log_id' => 2, 'size' => 1];
}
private static function restoring(array $current, array $snapshot, bool $unfiltered): DiviDesignRestoreAbility
{
Functions\when('current_user_can')->justReturn(true);
$writer = Mockery::mock(DesignSystemWriter::class);
$writer->allows('presets')->andReturn($current);
$writer->allows('canUseVisualBuilder')->andReturn(true);
$snapshots = Mockery::mock(SnapshotStore::class);
$snapshots->allows('get')->andReturn(self::snapshotOf($snapshot));
$snapshots->shouldNotReceive('save');
return new DiviDesignRestoreAbility(
$writer,
new DesignChange($snapshots, FakeChangeLog::forbidden()),
$snapshots,
static fn (): bool => $unfiltered
);
}
/** Final review I-2: markup in the snapshot or in the current inventory is lost for editors – refused. */
public function testEditorIsRefusedWhenTheSnapshotOrTheInventoryCarriesMarkup(): void
{
$plain = PresetBookTest::stored();
$markup = PresetBookTest::storedWithMarkup();
$older = $plain;
unset($older['module']['divi/button']['items']['cccccccccc']);
$input = ['snapshot_id' => 'presets-20261016T100000Z-aaaaaa'];
$inSnapshot = self::assertToolError(self::restoring($plain, $markup, false)->execute($input), 'forbidden');
self::assertStringContainsString('unfiltered_html', $inSnapshot['message']);
self::assertToolError(self::restoring($markup, $older, false)->execute($input), 'forbidden');
self::assertToolError(self::restoring($plain, $markup, true)->execute($input), 'confirm_required');
self::assertToolError(self::restoring($plain, $older, false)->execute($input), 'confirm_required');
}
/** T13: restoring needs the preset rights. */
public function testNeedsEditThemeOptions(): void
{
Functions\when('current_user_can')->justReturn(false);
$writer = Mockery::mock(DesignSystemWriter::class);
$writer->allows('canUseVisualBuilder')->andReturn(true);
$writer->shouldNotReceive('savePresets');
$snapshots = Mockery::mock(SnapshotStore::class);
$snapshots->shouldNotReceive('get');
$ability = new DiviDesignRestoreAbility($writer, new DesignChange($snapshots, FakeChangeLog::forbidden()), $snapshots);
$input = ['snapshot_id' => 'presets-20261016T100000Z-aaaaaa', 'confirm' => true, 'design_rev' => 'x'];
self::assertToolError($ability->permission($input), 'forbidden');
self::assertToolError($ability->execute($input), 'forbidden');
}
public function testUnknownSnapshotAndOtherAreas(): void public function testUnknownSnapshotAndOtherAreas(): void
{ {
Functions\when('current_user_can')->justReturn(true); Functions\when('current_user_can')->justReturn(true);
@@ -25,12 +25,17 @@ final class DiviPresetsDeleteAbilityTest extends TestCase
Functions\when('delete_option')->justReturn(true); Functions\when('delete_option')->justReturn(true);
} }
private function ability(UsageReport $report, ?DesignSystemWriter $writer = null, ?SnapshotStore $snapshots = null): DiviPresetsDeleteAbility private function ability(
{ UsageReport $report,
?DesignSystemWriter $writer = null,
?SnapshotStore $snapshots = null,
bool $unfiltered = true,
array $stored = []
): DiviPresetsDeleteAbility {
Functions\when('current_user_can')->justReturn(true); Functions\when('current_user_can')->justReturn(true);
if ($writer === null) { if ($writer === null) {
$writer = Mockery::mock(DesignSystemWriter::class); $writer = Mockery::mock(DesignSystemWriter::class);
$writer->allows('presets')->andReturn(PresetBookTest::stored()); $writer->allows('presets')->andReturn($stored === [] ? PresetBookTest::stored() : $stored);
$writer->allows('canUseVisualBuilder')->andReturn(true); $writer->allows('canUseVisualBuilder')->andReturn(true);
} }
$usages = Mockery::mock(GlobalUsages::class); $usages = Mockery::mock(GlobalUsages::class);
@@ -39,7 +44,8 @@ final class DiviPresetsDeleteAbilityTest extends TestCase
return new DiviPresetsDeleteAbility( return new DiviPresetsDeleteAbility(
$writer, $writer,
new DesignChange($snapshots ?? Mockery::mock(SnapshotStore::class), new FakeChangeLog(3)), new DesignChange($snapshots ?? Mockery::mock(SnapshotStore::class), new FakeChangeLog(3)),
$usages $usages,
static fn (): bool => $unfiltered
); );
} }
@@ -113,6 +119,22 @@ final class DiviPresetsDeleteAbilityTest extends TestCase
self::assertSame(['removed', 'default_cleared'], array_column($dry['diff'], 'action')); self::assertSame(['removed', 'default_cleared'], array_column($dry['diff'], 'action'));
} }
/** Final review I-2: deleting saves the whole inventory – other presets' HTML/CSS would be stripped. */
public function testEditorIsRefusedWhenOtherPresetsCarryMarkup(): void
{
$input = ['items' => [['module' => 'button', 'preset' => 'cccccccccc']]];
$markup = PresetBookTest::storedWithMarkup();
$editor = self::assertToolError(
$this->ability(self::unused(), null, null, false, $markup)->execute($input),
'forbidden'
);
self::assertStringContainsString('unfiltered_html', $editor['message']);
self::assertToolError($this->ability(self::unused(), null, null, true, $markup)->execute($input), 'confirm_required');
self::assertToolError($this->ability(self::unused(), null, null, false)->execute($input), 'confirm_required');
}
public function testUnknownPreset(): void public function testUnknownPreset(): void
{ {
self::assertToolError( self::assertToolError(
@@ -90,7 +90,8 @@ final class DiviPresetsSetAbilityTest extends TestCase
new DesignChange($this->snapshots, new FakeChangeLog(5)), new DesignChange($this->snapshots, new FakeChangeLog(5)),
$usages, $usages,
$loader ?? new DiviPageLoader(DiviFixtures::schema()), $loader ?? new DiviPageLoader(DiviFixtures::schema()),
$composer $composer,
static fn (): bool => $unfiltered
); );
} }
@@ -164,6 +165,45 @@ final class DiviPresetsSetAbilityTest extends TestCase
self::assertToolError($this->ability(true)->execute($input), 'confirm_required'); self::assertToolError($this->ability(true)->execute($input), 'confirm_required');
} }
/** Final review I-2: Divi would strip other presets' HTML/CSS when an editor saves – refused instead. */
public function testEditorIsRefusedWhenOtherPresetsCarryMarkup(): void
{
$this->stored = PresetBookTest::storedWithMarkup();
$input = ['items' => [[
'module' => 'button', 'preset' => 'CTA primär',
'attrs' => ['button.decoration.background' => ['desktop' => ['color' => '#222222']]],
]]];
$editor = self::assertToolError($this->ability(false)->execute($input), 'forbidden');
self::assertStringContainsString('unfiltered_html', $editor['message']);
self::assertStringContainsString('nichts wurde gespeichert', $editor['message']);
self::assertStringContainsString('Administrator', $editor['hint']);
self::assertToolError($this->ability(true)->execute($input), 'confirm_required');
}
/** The changed preset's own HTML/CSS would be stripped as well (closest safe variant of I-2). */
public function testEditorIsRefusedWhenTheChangedPresetCarriesMarkup(): void
{
$this->stored = PresetBookTest::storedWithMarkup();
unset($this->stored['group']['divi/font-body']['items']['gggggggggg']['styleAttrs']);
$editor = self::assertToolError($this->ability(false)->execute(['items' => [[
'module' => 'button', 'preset' => 'bbbbbbbbbb',
'attrs' => ['button.decoration.background' => ['desktop' => ['color' => '#222222']]],
]]]), 'forbidden');
self::assertStringContainsString('unfiltered_html', $editor['message']);
}
public function testEditorMayChangePresetsWhenNoneCarriesMarkup(): void
{
self::assertToolError($this->ability(false)->execute(['items' => [[
'module' => 'button', 'preset' => 'CTA primär',
'attrs' => ['button.decoration.background' => ['desktop' => ['color' => '#222222']]],
]]]), 'confirm_required');
self::assertToolError($this->ability(false)->execute(self::newButton()), 'confirm_required');
}
public function testFromNodeReadsThePage(): void public function testFromNodeReadsThePage(): void
{ {
$content = '<!-- wp:divi/section --><!-- wp:divi/row --><!-- wp:divi/column -->' $content = '<!-- wp:divi/section --><!-- wp:divi/row --><!-- wp:divi/column -->'
@@ -42,6 +42,31 @@ final class PresetBookTest extends TestCase
]; ];
} }
/** @return array<string, mixed> stored() with HTML in one preset and CSS with ">" in another. */
public static function storedWithMarkup(): array
{
$stored = self::stored();
$stored['module']['divi/button']['items']['bbbbbbbbbb']['attrs'] = [
'module' => ['advanced' => ['html' => ['desktop' => ['value' => ['htmlBefore' => '<b>x</b>']]]]],
];
$stored['group']['divi/font-body']['items']['gggggggggg']['styleAttrs'] = [
'css' => ['desktop' => ['value' => ['freeForm' => 'selector > p { color: red; }']]],
];
return $stored;
}
/** Final review I-2: presets whose HTML/CSS Divi's prepare_data() would strip for editors. */
public function testMarkupOutsideListsUntouchedPresetsWithHtmlOrCss(): void
{
$book = PresetBook::fromDivi(self::storedWithMarkup());
self::assertSame([], PresetBook::fromDivi(self::stored())->markupOutside([]));
self::assertSame(['bbbbbbbbbb', 'gggggggggg'], $book->markupOutside([]));
self::assertSame(['gggggggggg'], $book->markupOutside(['bbbbbbbbbb', 'aaaaaaaaaa']));
self::assertSame([], $book->markupOutside(['bbbbbbbbbb', 'gggggggggg']));
}
public function testFindsByIdOrUniqueName(): void public function testFindsByIdOrUniqueName(): void
{ {
$book = PresetBook::fromDivi(self::stored()); $book = PresetBook::fromDivi(self::stored());