From 585718cdff64c810c86f9304ad8e746bbb602277 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 21 Aug 2026 15:28:23 +0200 Subject: [PATCH 1/3] Use the shared AJAX security helper in AIOSEO tasks Five AIOSEO interactive task handlers verified that the plugin was active and that the nonce was valid, but did not check the user's capability: - Archive_Author - Archive_Date - Crawl_Settings_Feed_Comments - Crawl_Settings_Feed_Authors - Media_Pages They now call the existing verify_aioseo_ajax_security() helper, which runs the active check, the capability check and the nonce check together. Every other settings-changing interactive handler in the plugin already required manage_options, so this brings the AIOSEO handlers in line with the existing convention. Email_Sending::enqueue_scripts() also overrode its parent without carrying over the parent's capability guard. It now checks $this->capability_required(), which resolves to the capability the provider actually declares (manage_options here, inherited from Tasks) rather than a hardcoded literal, so it stays correct if a subclass overrides CAPABILITY. Also switch esc_attr() to esc_url() on three hrefs. esc_url is the correct escaper for a link target, since esc_attr does not validate the URL scheme. Verified: PHPCS clean, parallel-lint clean, PHPStan clean, PHPUnit 398 tests / 1203 assertions passing. Co-Authored-By: Claude Opus 5 (1M context) --- classes/suggested-tasks/providers/class-email-sending.php | 6 ++++++ classes/suggested-tasks/providers/class-hello-world.php | 2 +- classes/suggested-tasks/providers/class-sample-page.php | 2 +- classes/suggested-tasks/providers/class-tasks.php | 2 +- .../providers/integrations/aioseo/class-archive-author.php | 3 +-- .../providers/integrations/aioseo/class-archive-date.php | 3 +-- .../aioseo/class-crawl-settings-feed-authors.php | 3 +-- .../aioseo/class-crawl-settings-feed-comments.php | 3 +-- .../providers/integrations/aioseo/class-media-pages.php | 3 +-- 9 files changed, 14 insertions(+), 13 deletions(-) diff --git a/classes/suggested-tasks/providers/class-email-sending.php b/classes/suggested-tasks/providers/class-email-sending.php index 3225fff5a3..a1be12d193 100644 --- a/classes/suggested-tasks/providers/class-email-sending.php +++ b/classes/suggested-tasks/providers/class-email-sending.php @@ -189,6 +189,12 @@ protected function get_description( $task_data = [] ) { * @return void */ public function enqueue_scripts( $hook ) { + // Don't enqueue the script if the user lacks the capability required by this task, + // since the localized data contains a nonce. + if ( ! $this->capability_required() ) { + return; + } + // Enqueue the script only on Progress Planner and WP dashboard pages. if ( 'toplevel_page_progress-planner' !== $hook && 'index.php' !== $hook ) { return; diff --git a/classes/suggested-tasks/providers/class-hello-world.php b/classes/suggested-tasks/providers/class-hello-world.php index fe431f4199..b96320bec5 100644 --- a/classes/suggested-tasks/providers/class-hello-world.php +++ b/classes/suggested-tasks/providers/class-hello-world.php @@ -115,7 +115,7 @@ protected function get_description() { $content .= \sprintf( /* translators: %s: Link to the post. */ \esc_html__( 'On install, WordPress creates a "Hello World!" post. You can find yours at %s.', 'progress-planner' ), - '' . \esc_html( $hello_world_post_url ) . '', + '' . \esc_html( $hello_world_post_url ) . '', ); $content .= '

'; $content .= \esc_html__( 'This post does not add value to your website and solely exists to show what a post can look like. Therefore, "Hello World!" is not needed and should be deleted.', 'progress-planner' ); diff --git a/classes/suggested-tasks/providers/class-sample-page.php b/classes/suggested-tasks/providers/class-sample-page.php index e7ae313cf8..fef998f9fd 100644 --- a/classes/suggested-tasks/providers/class-sample-page.php +++ b/classes/suggested-tasks/providers/class-sample-page.php @@ -114,7 +114,7 @@ protected function get_description() { $content .= \sprintf( /* translators: %s: Link to the post. */ \esc_html__( 'On install, WordPress creates a "Sample Page" page. You can find yours at %s.', 'progress-planner' ), - '' . \esc_html( $sample_page_url ) . '', + '' . \esc_html( $sample_page_url ) . '', ); $content .= '

'; $content .= \esc_html__( 'This page does not add value to your website and solely exists to show what a page can look like. Therefore, "Sample Page" is not needed and should be deleted.', 'progress-planner' ); diff --git a/classes/suggested-tasks/providers/class-tasks.php b/classes/suggested-tasks/providers/class-tasks.php index 854bfebe99..94518540ef 100644 --- a/classes/suggested-tasks/providers/class-tasks.php +++ b/classes/suggested-tasks/providers/class-tasks.php @@ -736,7 +736,7 @@ public function get_task_actions( $data = [] ) { if ( $this->get_external_link_url() ) { $actions[] = [ 'priority' => 40, - 'html' => '' . \esc_html__( 'Why is this important?', 'progress-planner' ) . '', + 'html' => '' . \esc_html__( 'Why is this important?', 'progress-planner' ) . '', ]; } elseif ( isset( $data['content']['rendered'] ) && $data['content']['rendered'] !== '' && ! $this instanceof Tasks_Interactive ) { $actions[] = [ diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-archive-author.php b/classes/suggested-tasks/providers/integrations/aioseo/class-archive-author.php index 20cc606e49..4d5437cd6d 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-archive-author.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-archive-author.php @@ -153,8 +153,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); \aioseo()->options->searchAppearance->archives->author->show = false; // @phpstan-ignore-line diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-archive-date.php b/classes/suggested-tasks/providers/integrations/aioseo/class-archive-date.php index 2b60d55fc6..e0b124128b 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-archive-date.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-archive-date.php @@ -140,8 +140,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); \aioseo()->options->searchAppearance->archives->date->show = false; // @phpstan-ignore-line diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-authors.php b/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-authors.php index 74297316e5..950eb99b44 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-authors.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-authors.php @@ -150,8 +150,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); \aioseo()->options->searchAppearance->advanced->crawlCleanup->feeds->authors = false; // @phpstan-ignore-line diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-comments.php b/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-comments.php index b942ac4a4c..528174c5aa 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-comments.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-crawl-settings-feed-comments.php @@ -119,8 +119,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); // Global comment feed. if ( \aioseo()->options->searchAppearance->advanced->crawlCleanup->feeds->globalComments ) { // @phpstan-ignore-line diff --git a/classes/suggested-tasks/providers/integrations/aioseo/class-media-pages.php b/classes/suggested-tasks/providers/integrations/aioseo/class-media-pages.php index 93b22f6461..cc74ba42b1 100644 --- a/classes/suggested-tasks/providers/integrations/aioseo/class-media-pages.php +++ b/classes/suggested-tasks/providers/integrations/aioseo/class-media-pages.php @@ -126,8 +126,7 @@ public function print_popover_form_contents() { * @return void */ public function handle_interactive_task_specific_submit() { - $this->verify_aioseo_active_or_fail(); - $this->verify_nonce_or_fail(); + $this->verify_aioseo_ajax_security(); \aioseo()->dynamicOptions->searchAppearance->postTypes->attachment->redirectAttachmentUrls = 'attachment'; // @phpstan-ignore-line From 5b9fbca39eae2d29239505c318527e3b09824b33 Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 21 Aug 2026 15:45:28 +0200 Subject: [PATCH 2/3] Bind term task handlers to the term the task suggested Remove_Terms_Without_Posts and Update_Term_Description acted on whatever term_id/taxonomy the request supplied, without checking it against the term the task actually suggested. The relevant constraints already existed in the task generation path, they just were not applied when acting on a request: maybe_remove_irrelevant_tasks() skips non-public taxonomies and drops tasks once a term has more than MIN_POSTS posts. Both handlers now, after the existing capability and nonce checks: - reject non-public taxonomies, - require a task from the same provider that targets this exact term_id/taxonomy pair, - and, for deletion, re-check the post count at submit time, since the term may have gained posts after the task was created. Binding rather than raising the required capability keeps the feature working for its intended audience: these tasks are meant for Editors, and the JS already submits the task's own target_term_id and target_taxonomy, so the legitimate flow is unchanged. Adds regression tests covering both the rejected and the permitted paths. Verified that the negative tests fail when the new guards are removed, and that the positive tests still pass, so they exercise the behaviour rather than passing vacuously. Verified: PHPCS clean, PHPStan clean, PHPUnit 404 tests / 1216 assertions passing. Co-Authored-By: Claude Opus 5 (1M context) --- .../class-remove-terms-without-posts.php | 39 +++ .../class-update-term-description.php | 32 +++ .../phpunit/test-class-term-task-binding.php | 241 ++++++++++++++++++ 3 files changed, 312 insertions(+) create mode 100644 tests/phpunit/test-class-term-task-binding.php diff --git a/classes/suggested-tasks/providers/class-remove-terms-without-posts.php b/classes/suggested-tasks/providers/class-remove-terms-without-posts.php index 79c8cf9f2a..12bd8b1323 100644 --- a/classes/suggested-tasks/providers/class-remove-terms-without-posts.php +++ b/classes/suggested-tasks/providers/class-remove-terms-without-posts.php @@ -453,6 +453,25 @@ public function handle_interactive_task_submit() { \wp_send_json_error( [ 'message' => \esc_html__( 'Term not found.', 'progress-planner' ) ] ); } + // Only public taxonomies are tracked by this task, mirroring the check in + // maybe_remove_irrelevant_tasks(). + $taxonomy_object = \get_taxonomy( $taxonomy ); + if ( ! $taxonomy_object || ! $taxonomy_object->public ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to delete terms.', 'progress-planner' ) ] ); + } + + // Bind the request to a task that actually suggested this term, so this + // handler cannot be repurposed to delete arbitrary terms. + if ( ! $this->has_task_for_term( $term_id, $taxonomy ) ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to delete terms.', 'progress-planner' ) ] ); + } + + // Re-check the post count at deletion time: the term may have gained posts + // after the task was created. + if ( $term->count > self::MIN_POSTS ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'This term is not empty and cannot be deleted.', 'progress-planner' ) ] ); + } + // Delete the term. $result = \wp_delete_term( $term_id, $taxonomy ); @@ -462,4 +481,24 @@ public function handle_interactive_task_submit() { \wp_send_json_success( [ 'message' => \esc_html__( 'Term deleted successfully.', 'progress-planner' ) ] ); } + + /** + * Check whether a task from this provider targets the given term. + * + * @param int $term_id The term ID. + * @param string $taxonomy The taxonomy. + * + * @return bool + */ + protected function has_task_for_term( $term_id, $taxonomy ) { + foreach ( \progress_planner()->get_suggested_tasks_db()->get_tasks_by( [ 'provider_id' => $this->get_provider_id() ] ) as $task ) { + if ( (int) $task->target_term_id === (int) $term_id + && (string) $task->target_taxonomy === (string) $taxonomy + ) { + return true; + } + } + + return false; + } } diff --git a/classes/suggested-tasks/providers/class-update-term-description.php b/classes/suggested-tasks/providers/class-update-term-description.php index 455a5675b7..fc27fef86f 100644 --- a/classes/suggested-tasks/providers/class-update-term-description.php +++ b/classes/suggested-tasks/providers/class-update-term-description.php @@ -445,6 +445,18 @@ public function handle_interactive_task_submit() { \wp_send_json_error( [ 'message' => \esc_html__( 'Term not found.', 'progress-planner' ) ] ); } + // Only public taxonomies are tracked by this task. + $taxonomy_object = \get_taxonomy( $taxonomy ); + if ( ! $taxonomy_object || ! $taxonomy_object->public ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to update terms.', 'progress-planner' ) ] ); + } + + // Bind the request to a task that actually suggested this term, so this + // handler cannot be repurposed to edit arbitrary terms. + if ( ! $this->has_task_for_term( $term_id, $taxonomy ) ) { + \wp_send_json_error( [ 'message' => \esc_html__( 'You do not have permission to update terms.', 'progress-planner' ) ] ); + } + // Update the term description. $result = \wp_update_term( $term_id, @@ -460,4 +472,24 @@ public function handle_interactive_task_submit() { \wp_send_json_success( [ 'message' => \esc_html__( 'Term description updated successfully.', 'progress-planner' ) ] ); } + + /** + * Check whether a task from this provider targets the given term. + * + * @param int $term_id The term ID. + * @param string $taxonomy The taxonomy. + * + * @return bool + */ + protected function has_task_for_term( $term_id, $taxonomy ) { + foreach ( \progress_planner()->get_suggested_tasks_db()->get_tasks_by( [ 'provider_id' => $this->get_provider_id() ] ) as $task ) { + if ( (int) $task->target_term_id === (int) $term_id + && (string) $task->target_taxonomy === (string) $taxonomy + ) { + return true; + } + } + + return false; + } } diff --git a/tests/phpunit/test-class-term-task-binding.php b/tests/phpunit/test-class-term-task-binding.php new file mode 100644 index 0000000000..b7c97af85f --- /dev/null +++ b/tests/phpunit/test-class-term-task-binding.php @@ -0,0 +1,241 @@ +_setRole( 'editor' ); + } + + /** + * Clean up. + */ + public function tear_down() { + unset( $_REQUEST['term_id'], $_REQUEST['taxonomy'], $_REQUEST['description'], $_REQUEST['nonce'] ); + $_POST = []; + parent::tear_down(); + } + + /** + * Populate the request with a signed payload. + * + * @param int $term_id The term ID. + * @param string $taxonomy The taxonomy. + * @param string $description Optional description. + * + * @return void + */ + private function set_request( $term_id, $taxonomy, $description = null ) { + $_REQUEST['term_id'] = $term_id; + $_REQUEST['taxonomy'] = $taxonomy; + $_REQUEST['nonce'] = \wp_create_nonce( 'progress_planner' ); + + if ( null !== $description ) { + $_REQUEST['description'] = $description; + } + + // The handlers read from $_POST; check_ajax_referer reads $_REQUEST. + $_POST = $_REQUEST; + } + + /** + * Run a handler and return the decoded JSON response. + * + * @param object $provider The task provider. + * + * @return array|null The decoded response. + */ + private function get_response( $provider ) { + // WordPress Core's _handleAjax() starts output buffering before calling + // AJAX actions. We do the same since we call the method directly. + \ini_set( 'implicit_flush', false ); // phpcs:ignore WordPress.PHP.IniSet.Risky + \ob_start(); + + try { + $provider->handle_interactive_task_submit(); + } catch ( \WPAjaxDieContinueException $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch + // Expected: wp_send_json_* calls wp_die(). + } + + return \json_decode( $this->_last_response, true ); + } + + /** + * Create a task from a provider that targets a specific term. + * + * @param object $provider The task provider. + * @param int $term_id The term ID. + * @param string $taxonomy The taxonomy. + * + * @return void + */ + private function create_task_for_term( $provider, $term_id, $taxonomy ) { + \progress_planner()->get_suggested_tasks_db()->add( + [ + 'post_title' => 'Term task', + 'provider_id' => $provider->get_provider_id(), + 'target_term_id' => $term_id, + 'target_taxonomy' => $taxonomy, + ] + ); + } + + /** + * An Editor cannot delete an unrelated term that no task suggested. + * + * This is the privilege-escalation case: wp_delete_term() performs no + * capability check of its own, so without binding an Editor could delete + * any term in any taxonomy. + * + * @return void + */ + public function test_delete_rejects_term_without_matching_task() { + $term_id = $this->factory->term->create( [ 'taxonomy' => 'category' ] ); + + $this->set_request( $term_id, 'category' ); + $response = $this->get_response( new Remove_Terms_Without_Posts() ); + + $this->assertFalse( $response['success'], 'Deletion should be rejected.' ); + + $term = \get_term( $term_id, 'category' ); + $this->assertFalse( \is_wp_error( $term ), 'Term should not have been deleted.' ); + $this->assertNotNull( $term, 'Term should not have been deleted.' ); + } + + /** + * An Editor cannot rewrite the description of a term no task suggested. + * + * @return void + */ + public function test_update_description_rejects_term_without_matching_task() { + $term_id = $this->factory->term->create( + [ + 'taxonomy' => 'category', + 'description' => 'Original description.', + ] + ); + + $this->set_request( $term_id, 'category', 'Injected description.' ); + $response = $this->get_response( new Update_Term_Description() ); + + $this->assertFalse( $response['success'], 'Update should be rejected.' ); + + $term = \get_term( $term_id, 'category' ); + $this->assertSame( 'Original description.', $term->description, 'Description should be unchanged.' ); + } + + /** + * A non-public taxonomy is rejected even when a task exists for the term. + * + * @return void + */ + public function test_delete_rejects_non_public_taxonomy() { + \register_taxonomy( 'prpl_private_tax', 'post', [ 'public' => false ] ); + $term_id = $this->factory->term->create( [ 'taxonomy' => 'prpl_private_tax' ] ); + + $provider = new Remove_Terms_Without_Posts(); + $this->create_task_for_term( $provider, $term_id, 'prpl_private_tax' ); + + $this->set_request( $term_id, 'prpl_private_tax' ); + $response = $this->get_response( $provider ); + + $this->assertFalse( $response['success'], 'Non-public taxonomy should be rejected.' ); + $this->assertFalse( \is_wp_error( \get_term( $term_id, 'prpl_private_tax' ) ), 'Term should not have been deleted.' ); + + \unregister_taxonomy( 'prpl_private_tax' ); + } + + /** + * A term that a task legitimately suggested can still be deleted. + * + * Guards against the binding being too strict and breaking the feature. + * + * @return void + */ + public function test_delete_allows_term_suggested_by_a_task() { + $term_id = $this->factory->term->create( [ 'taxonomy' => 'category' ] ); + + $provider = new Remove_Terms_Without_Posts(); + $this->create_task_for_term( $provider, $term_id, 'category' ); + + $this->set_request( $term_id, 'category' ); + $response = $this->get_response( $provider ); + + $this->assertTrue( $response['success'], 'Deletion of a suggested term should succeed.' ); + + $term = \get_term( $term_id, 'category' ); + $this->assertTrue( null === $term || \is_wp_error( $term ), 'Term should have been deleted.' ); + } + + /** + * A description update for a legitimately suggested term still works. + * + * @return void + */ + public function test_update_description_allows_term_suggested_by_a_task() { + $term_id = $this->factory->term->create( + [ + 'taxonomy' => 'category', + 'description' => 'Original description.', + ] + ); + + $provider = new Update_Term_Description(); + $this->create_task_for_term( $provider, $term_id, 'category' ); + + $this->set_request( $term_id, 'category', 'A better description.' ); + $response = $this->get_response( $provider ); + + $this->assertTrue( $response['success'], 'Update of a suggested term should succeed.' ); + + $term = \get_term( $term_id, 'category' ); + $this->assertSame( 'A better description.', $term->description, 'Description should have been updated.' ); + } + + /** + * A suggested term that has since gained posts is not deleted. + * + * @return void + */ + public function test_delete_rejects_suggested_term_that_gained_posts() { + $term_id = $this->factory->term->create( [ 'taxonomy' => 'category' ] ); + + $provider = new Remove_Terms_Without_Posts(); + $this->create_task_for_term( $provider, $term_id, 'category' ); + + // Give the term enough posts to exceed MIN_POSTS. + foreach ( [ 1, 2 ] as $ignored ) { + $post_id = $this->factory->post->create(); + \wp_set_object_terms( $post_id, [ $term_id ], 'category', true ); + } + + $this->set_request( $term_id, 'category' ); + $response = $this->get_response( $provider ); + + $this->assertFalse( $response['success'], 'A term with posts should not be deleted.' ); + $this->assertFalse( \is_wp_error( \get_term( $term_id, 'category' ) ), 'Term should not have been deleted.' ); + } +} From e89ecf83e5ab5b0f7c2dcb27e248f728cc70a7be Mon Sep 17 00:00:00 2001 From: Filip Ilic Date: Fri, 21 Aug 2026 16:04:11 +0200 Subject: [PATCH 3/3] Build media preview via DOM instead of an HTML string The site icon and Yoast organization logo pickers assembled their preview by concatenating attachment metadata into an HTML string and assigning it to innerHTML. Both now build the node with createElement and property assignment, so no HTML string is parsed and attachment metadata is always handled as an attribute value rather than as markup. This follows the same reasoning as the existing textContent usage in updateTaskTitle(). Verified with jsdom that metadata containing quote characters round-trips as a value and does not become markup. Note: the Yoast logo picker's alt fallback still reads "Site icon preview", which looks like a copy-paste artifact. Left as-is to avoid mixing a user-visible string change into this commit. Verified: JS lint clean, PHPCS clean, PHPUnit 404 tests / 1216 assertions passing. Co-Authored-By: Claude Opus 5 (1M context) --- assets/js/recommendations/core-siteicon.js | 16 ++++++++++------ .../recommendations/yoast-organization-logo.js | 16 ++++++++++------ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/assets/js/recommendations/core-siteicon.js b/assets/js/recommendations/core-siteicon.js index b91380f841..3b5137e4f5 100644 --- a/assets/js/recommendations/core-siteicon.js +++ b/assets/js/recommendations/core-siteicon.js @@ -159,12 +159,16 @@ ? attachment.sizes.thumbnail.url : attachment.url; - this.elements.preview.innerHTML = - '' + - ( attachment.alt || 'Site icon preview' ) + - ''; + // Build the node instead of interpolating into an HTML string: the + // alt text is free-text set by any upload-capable user, and a value + // like `" onerror="` would otherwise break out of the attribute. + const img = document.createElement( 'img' ); + img.src = imageUrl; + img.alt = attachment.alt || 'Site icon preview'; + img.style.cssText = + 'max-width: 150px; height: auto; border-radius: 4px; border: 1px solid #ddd;'; + + this.elements.preview.replaceChildren( img ); } /** diff --git a/assets/js/recommendations/yoast-organization-logo.js b/assets/js/recommendations/yoast-organization-logo.js index dc70f09c47..e2c6cd0403 100644 --- a/assets/js/recommendations/yoast-organization-logo.js +++ b/assets/js/recommendations/yoast-organization-logo.js @@ -164,12 +164,16 @@ ? attachment.sizes.thumbnail.url : attachment.url; - this.elements.preview.innerHTML = - '' + - ( attachment.alt || 'Site icon preview' ) + - ''; + // Build the node instead of interpolating into an HTML string: the + // alt text is free-text set by any upload-capable user, and a value + // like `" onerror="` would otherwise break out of the attribute. + const img = document.createElement( 'img' ); + img.src = imageUrl; + img.alt = attachment.alt || 'Site icon preview'; + img.style.cssText = + 'max-width: 150px; height: auto; border-radius: 4px; border: 1px solid #ddd;'; + + this.elements.preview.replaceChildren( img ); } /**