fix(audit L5): et_theme_options unter der gemeinsamen Sperre aus et_divi neu laden, bevor Farben oder Theme-Optionen schreiben

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Jan-Niclas Loosen
2026-10-19 23:52:38 +02:00
co-authored by Claude Sonnet 5.5
parent b59a0c045f
commit 1b3ef552e7
4 changed files with 41 additions and 8 deletions
@@ -45,7 +45,7 @@ class GlobalColorsAdapter implements DiviAdapter
new Call(GlobalData::class . '::sanitize_global_colors_data', 1), new Call(GlobalData::class . '::sanitize_global_colors_data', 1),
new Call(GlobalData::class . '::set_global_colors', 2), new Call(GlobalData::class . '::set_global_colors', 2),
], ],
keys: [self::OPTIONS], keys: [self::OPTIONS, 'et_theme_options'],
features: [DiviFeature::ColorWrite] features: [DiviFeature::ColorWrite]
); );
} }
@@ -83,6 +83,8 @@ class GlobalColorsAdapter implements DiviAdapter
throw new RuntimeException('global colors refused: Divi dropped ' . implode(', ', $dropped)); throw new RuntimeException('global colors refused: Divi dropped ' . implode(', ', $dropped));
} }
// Audit L5: set_global_colors() ends in et_update_option(), which writes Divi's request global back whole.
$GLOBALS['et_theme_options'] = get_option('et_divi');
GlobalData::set_global_colors($clean, ! $replace); GlobalData::set_global_colors($clean, ! $replace);
} }
} }
@@ -21,9 +21,9 @@ defined('ABSPATH') || exit;
* them raw from get_option('et_divi'), sends a value through the customizer's own sanitizer of its setting, and * them raw from get_option('et_divi'), sends a value through the customizer's own sanitizer of its setting, and
* writes the way a customizer save ends – et_update_option()/et_delete_option() per key, then the static CSS and the * writes the way a customizer save ends – et_update_option()/et_delete_option() per key, then the static CSS and the
* builder caches (research A.2/A.3). Limits (W13): et_update_option() writes the whole et_divi array from Divi's * builder caches (research A.2/A.3). Limits (W13): et_update_option() writes the whole et_divi array from Divi's
* request global, so a change another process made after this request loaded it is lost; design_rev and the lock of * request global; write() reloads it from et_divi first, under the lock colors and theme options share (audit L5), so
* the pipeline narrow that window. remove_static_resources() does nothing without a logged-in user. Not final so the * only Divi's own saves can still race it. remove_static_resources() does nothing without a logged-in user. Not
* theme option area can be tested with a mock. * final so the theme option area can be tested with a mock.
*/ */
class ThemeOptionsAdapter implements DiviAdapter class ThemeOptionsAdapter implements DiviAdapter
{ {
@@ -74,7 +74,7 @@ class ThemeOptionsAdapter implements DiviAdapter
new Call('et_pb_force_regenerate_templates', 0), new Call('et_pb_force_regenerate_templates', 0),
new Call('et_pb_is_allowed', 1), new Call('et_pb_is_allowed', 1),
], ],
keys: [self::OPTIONS], keys: [self::OPTIONS, 'et_theme_options'],
mirrors: [ mirrors: [
// shortcut: the fingerprint covers the whole customizer function (about 4,000 lines) and changes with // shortcut: the fingerprint covers the whole customizer function (about 4,000 lines) and changes with
// every customizer edit; switch to a narrower excerpt when it fires too often. // every customizer edit; switch to a narrower excerpt when it fires too often.
@@ -150,6 +150,9 @@ class ThemeOptionsAdapter implements DiviAdapter
throw new InvalidArgumentException('not a theme option of the whitelist: ' . implode(', ', $unknown)); throw new InvalidArgumentException('not a theme option of the whitelist: ' . implode(', ', $unknown));
} }
// Audit L5: et_update_option() writes Divi's request global back whole; under the shared et_divi lock reload
// it so another writer's change (colors) made since this request began is not reverted.
$GLOBALS['et_theme_options'] = get_option('et_divi');
foreach ($set as $key => $value) { foreach ($set as $key => $value) {
et_update_option($key, $value); et_update_option($key, $value);
} }
@@ -39,7 +39,7 @@ final class GlobalColorsAdapterTest extends TestCase
array_map(static fn (Call $call): string => $call->label(), $manifest->calls) array_map(static fn (Call $call): string => $call->label(), $manifest->calls)
); );
self::assertSame([0, 1, 2], array_map(static fn (Call $call): ?int => $call->args, $manifest->calls)); self::assertSame([0, 1, 2], array_map(static fn (Call $call): ?int => $call->args, $manifest->calls));
self::assertSame(['et_divi'], $manifest->keys); self::assertSame(['et_divi', 'et_theme_options'], $manifest->keys);
self::assertSame([DiviFeature::ColorWrite], $manifest->features); self::assertSame([DiviFeature::ColorWrite], $manifest->features);
} }
@@ -94,11 +94,21 @@ final class GlobalColorsAdapterTest extends TestCase
$divi = Mockery::mock('alias:' . self::GLOBAL_DATA); $divi = Mockery::mock('alias:' . self::GLOBAL_DATA);
$divi->expects('sanitize_global_colors_data')->twice()->with($entries)->andReturn($clean); $divi->expects('sanitize_global_colors_data')->twice()->with($entries)->andReturn($clean);
// Normal write: merge ("already sanitized" = true); restore: replace all (false). // Normal write: merge ("already sanitized" = true); restore: replace all (false).
$divi->expects('set_global_colors')->once()->with($clean, true)->ordered(); // Audit L5: Divi's request global is reloaded from et_divi before the write.
$GLOBALS['et_theme_options'] = ['stale' => true];
Functions\when('get_option')->alias(static fn (string $name): mixed => $name === 'et_divi' ? ['fresh' => 1] : false);
$seen = [];
$divi->expects('set_global_colors')->once()->with($clean, true)->ordered()
->andReturnUsing(static function () use (&$seen): void {
$seen[] = $GLOBALS['et_theme_options'];
});
$divi->expects('set_global_colors')->once()->with($clean, false)->ordered(); $divi->expects('set_global_colors')->once()->with($clean, false)->ordered();
(new GlobalColorsAdapter())->save($entries, false); (new GlobalColorsAdapter())->save($entries, false);
(new GlobalColorsAdapter())->save($entries, true); (new GlobalColorsAdapter())->save($entries, true);
self::assertSame([['fresh' => 1]], $seen);
unset($GLOBALS['et_theme_options']);
} }
/** @runInSeparateProcess */ /** @runInSeparateProcess */
@@ -54,7 +54,7 @@ final class ThemeOptionsAdapterTest extends TestCase
[1, 1, 1, 1, 1, 2, 1, 2, 0, 1], [1, 1, 1, 1, 1, 2, 1, 2, 0, 1],
array_map(static fn (Call $call): ?int => $call->args, $manifest->calls) array_map(static fn (Call $call): ?int => $call->args, $manifest->calls)
); );
self::assertSame(['et_divi'], $manifest->keys); self::assertSame(['et_divi', 'et_theme_options'], $manifest->keys);
self::assertSame([DiviFeature::ThemeOptionWrite], $manifest->features); self::assertSame([DiviFeature::ThemeOptionWrite], $manifest->features);
self::assertCount(1, $manifest->mirrors); self::assertCount(1, $manifest->mirrors);
@@ -196,6 +196,18 @@ final class ThemeOptionsAdapterTest extends TestCase
$log[] = 'regenerate templates after ' . count(\ET_Core_PageResource::$removed) . ' static CSS clear'; $log[] = 'regenerate templates after ' . count(\ET_Core_PageResource::$removed) . ' static CSS clear';
}); });
\ET_Core_PageResource::$removed = []; \ET_Core_PageResource::$removed = [];
// Audit L5: Divi's request global may be stale; et_update_option() writes it back whole.
$GLOBALS['et_theme_options'] = ['stale' => true];
$seen = [];
Functions\when('et_update_option')->alias(static function (string $key, mixed $value) use (
&$stored,
&$log,
&$seen
): void {
$seen[] = $GLOBALS['et_theme_options'];
$stored[$key] = $value;
$log[] = 'update ' . $key;
});
$result = (new ThemeOptionsAdapter())->write( $result = (new ThemeOptionsAdapter())->write(
['body_font_size' => 16, 'body_font_height' => 1.8], ['body_font_size' => 16, 'body_font_height' => 1.8],
@@ -211,6 +223,12 @@ final class ThemeOptionsAdapterTest extends TestCase
$log $log
); );
self::assertSame([['all', 'all', false]], \ET_Core_PageResource::$removed, 'like a customizer save'); self::assertSame([['all', 'all', false]], \ET_Core_PageResource::$removed, 'like a customizer save');
self::assertSame(
['body_font_size' => 14, 'heading_font' => 'Roboto', 'accent_color' => '#ff0000'],
$seen[0],
'et_theme_options is reloaded from et_divi before the first write'
);
unset($GLOBALS['et_theme_options']);
self::assertSame('#ff0000', $stored['accent_color'], 'other theme options stay'); self::assertSame('#ff0000', $stored['accent_color'], 'other theme options stay');
} }