Merge pull request #17034 from ckeditor/revert-16878-ci/16870-sanitiz…

…e-html

Other (core,html-embed): Reverted recent change to move `config.htmlEmbed.sanitizeHtml` to a top-level config property (`config.sanitizeHtml`). Starting from v43.1.0 `config.htmlEmbed.sanitizeHtml` is no longer deprecated.

MINOR BREAKING CHANGE: In v43.0.0 we made a decision to move `config.htmlEmbed.sanitizeHtml` to a top-level property `config.sanitizeHtml`, so it can be used by multiple features (HTML Embed and Merge Fields). However, we realized that it was a wrong decision to expose such a sensitive property in a top-level config property and decided to revert that change. Starting with v43.1.0 you should again use `config.htmlEmbed.sanitizeHtml` and `config.mergeFields.sanitizeHtml`. The editor will throw an error if `config.sanitizeHtml` is used.

CHANGELOG.md

@@ -1,7 +1,7 @@
 Changelog
 =========
 
-## [43.1.0](https://github.com/ckeditor/ckeditor5/compare/v43.0.0...v43.1.0) (September 4, 2024)
+## [43.1.0](https://github.com/ckeditor/ckeditor5/compare/v43.0.0...v43.1.0) (September 5, 2024)
 
 We are happy to announce the release of CKEditor 5 v43.1.0.
 
@@ -20,6 +20,8 @@ The full list of enhancements can be found below.
 
 ### MINOR BREAKING CHANGES [ℹ️](https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/versioning-policy.html#major-and-minor-breaking-changes)
 
+* **Reverted `config.sanitizeHtml`.** In v43.0.0 we made a decision to move `config.htmlEmbed.sanitizeHtml` to a top-level property `config.sanitizeHtml`. However, we realized that it was a wrong decision to expose such a sensitive property in a top-level configuration property. Starting with v43.1.0 you should again use `config.htmlEmbed.sanitizeHtml` and/or `config.mergeFields.sanitizeHtml`. The editor will throw an error if `config.sanitizeHtml` is used. See the {@link  
+updating/update-to-43#reverted-recently-introduced-configsanitizehtml migration guide} for additional context behind this decision.
 * **[ai](https://www.npmjs.com/package/@ckeditor/ckeditor5-ai)**: The structure and presentation of the list of AI commands in the toolbar have changed (a flat filtered list is now a nested menu). Additionally, if your integration customizes this user interface, please ensure your integration code is up-to-date.
 * **[ui](https://www.npmjs.com/package/@ckeditor/ckeditor5-ui)**: The default `[aria-label]` provided by `InlineEditableUIView` is now `'Rich Text Editor. Editing area: [root name]'` (previously: `'Editor editing area: [root name]'`). You can use the `options.label` constructor property to adjust the label.
 
@@ -55,6 +57,9 @@ The full list of enhancements can be found below.
 
 * **[ai](https://www.npmjs.com/package/@ckeditor/ckeditor5-ai)**: The AI Assistant pre-defined commands toolbar dropdown will now use a new nested menu component instead of the flat list component.
 * **[comments](https://www.npmjs.com/package/@ckeditor/ckeditor5-comments)**: Moved <kbd>Ctrl</kbd>+<kbd>Shift</kbd>+<kbd>E</kbd> and <kbd>Esc</kbd> key handling code from individual features to the `Annotations` plugin to simplify the logic.
+* **[core](https://www.npmjs.com/package/@ckeditor/ckeditor5-core)**: Reverted recent change to move `config.htmlEmbed.sanitizeHtml` to a top-level config property (`config.sanitizeHtml`). `config.sanitizeHtml` is no longer available and using it will throw an error.
+* **[html-embed](https://www.npmjs.com/package/@ckeditor/ckeditor5-html-embed)**: Reverted recent change to move `config.htmlEmbed.sanitizeHtml` to a top-level config property (`config.sanitizeHtml`). Starting from v43.1.0 `config.htmlEmbed.sanitizeHtml` is no longer deprecated.
+* **[merge-fields](https://www.npmjs.com/package/@ckeditor/ckeditor5-merge-fields)**: Introduced `config.mergeFields.sanitizeHtml` config property. Use it instead of `config.sanitizeHtml`. `config.sanitizeHtml` is no longer available and using it will throw an error.
 * **[track-changes](https://www.npmjs.com/package/@ckeditor/ckeditor5-track-changes)**: Moved <kbd>Ctrl</kbd>+<kbd>Shift</kbd>+<kbd>E</kbd> and <kbd>Esc</kbd> key handling code from individual features to the `Annotations` plugin to simplify the logic.
 * **[typing](https://www.npmjs.com/package/@ckeditor/ckeditor5-typing)**: The package exports now the `TextTransformationConfig` type. ([commit](https://github.com/ckeditor/ckeditor5/commit/0d8adcef6eaf79fd35cc2647159ceeedd9e1a0d8))
 * Updated translations. ([commit](https://github.com/ckeditor/ckeditor5/commit/c394fd71229002ff813a16fdb78ec1e9ca6985da))

docs/getting-started/setup/csp.md

@@ -1,7 +1,7 @@
 ---
 category: setup
 meta-title: Content Security Policy | CKEditor 5 documentation
-order: 120
+order: 110
 ---
 
 # Content Security Policy

docs/getting-started/setup/optimizing-build-size.md

@@ -1,7 +1,7 @@
 ---
 category: setup
 meta-title: Optimizing build size | CKEditor 5 documentation
-order: 130
+order: 120
 modified_at: 2024-06-25
 ---
 

docs/updating/update-to-43.md

@@ -16,7 +16,17 @@ modified_at: 2024-07-31
 
 ## Update to CKEditor 5 v43.1.0
 
-_Released on September 4, 2024._
+_Released on September 5, 2024._
+
+### Reverted recently introduced `config.sanitizeHtml`
+
+In v43.0.0 we made a decision to move {@link module:html-embed/htmlembedconfig~HtmlEmbedConfig#sanitizeHtml `config.htmlEmbed.sanitizeHtml`} to a top-level property `config.sanitizeHtml`, so it could be used by multiple features (HTML embed, merge fields, and possibly other in future).
+
+However, we realized that it was a wrong decision to expose such a sensitive property in a top-level configuration property. We are worried that integrators may be confused and incorrectly assume that this callback would sanitize the entire output from CKEditor. Therefore, we decided to revert that change, so the sanitization callback is related strictly with the features that use it.
+
+Starting with v43.1.0, you should again use {@link module:html-embed/htmlembedconfig~HtmlEmbedConfig#sanitizeHtml `config.htmlEmbed.sanitizeHtml`} and newly introduced {@link module:merge-fields/mergefieldsconfig~MergeFieldsConfig#sanitizeHtml `config.mergeFields.sanitizeHtml`}. The editor will throw an error if `config.sanitizeHtml` is used.
+
+Note: CKEditor 5, by default, prevents execution of scripts in the editor content, while the content is being edited inside the editor. However, there are features (such as General HTML support or HTML embed) that can be configured to make CKEditor 5 produce HTML output that contains executable scripts. Please remember, that CKEditor 5 is a frontend component working in a browser. As an integrator, it is your responsibility to sanitize the content before it is displayed on your website or on other potentially vulnerable medium.
 
 ### Table and cell border settings update
 

packages/ckeditor5-clipboard/tests/manual/dragdrop-blocks.js

@@ -175,9 +175,9 @@ ClassicEditor
 			}
 		},
 		htmlEmbed: {
-			showPreviews: true
+			showPreviews: true,
+			sanitizeHtml: html => ( { html, hasChange: false } )
 		},
-		sanitizeHtml: html => ( { html, hasChange: false } ),
 		list: {
 			properties: {
 				styles: true,

packages/ckeditor5-core/src/editor/editor.ts

@@ -11,7 +11,6 @@ import {
 	Config,
 	CKEditorError,
 	ObservableMixin,
-	logWarning,
 	type Locale,
 	type LocaleTranslate,
 	type ObservableChangeEvent
@@ -281,6 +280,17 @@ export default abstract class Editor extends /* #__PURE__ */ ObservableMixin() {
 	constructor( config: EditorConfig = {} ) {
 		super();
 
+		if ( 'sanitizeHtml' in config ) {
+			/**
+			 * Configuration property `config.sanitizeHtml` was removed in CKEditor version 43.1.0 and is no longer supported.
+			 *
+			 * Please use `config.htmlEmbed.sanitizeHtml` and/or `config.mergeFields.sanitizeHtml` instead.
+			 *
+			 * @error editor-config-sanitizehtml-not-supported
+			 */
+			throw new CKEditorError( 'editor-config-sanitizehtml-not-supported' );
+		}
+
 		const constructor = this.constructor as typeof Editor;
 
 		// We don't pass translations to the config, because its behavior of splitting keys
@@ -301,23 +311,6 @@ export default abstract class Editor extends /* #__PURE__ */ ObservableMixin() {
 		this.config = new Config<EditorConfig>( rest, defaultConfig );
 		this.config.define( 'plugins', availablePlugins );
 		this.config.define( this._context._getEditorConfig() );
-		this.config.define( 'sanitizeHtml', function( rawHtml ) {
-			/**
-			 * One of the editor features directly inserts unsanitized HTML code into the editor.
-			 * It is strongly recommended to define a sanitize function that will clean up the input HTML
-			 * in order to avoid XSS vulnerability.
-			 *
-			 * For a detailed overview, check the {@glink getting-started/setup/html-security "HTML security"} guide.
-			 *
-			 * @error provide-sanitize-function
-			 */
-			logWarning( 'provide-sanitize-function' );
-
-			return {
-				html: rawHtml,
-				hasChanged: false
-			};
-		} );
 
 		this.plugins = new PluginCollection<Editor>( this, availablePlugins, this._context.plugins );
 

packages/ckeditor5-core/src/editor/editorconfig.ts

@@ -822,44 +822,6 @@ export interface EditorConfig {
 	 */
 	translations?: ArrayOrItem<Translations>;
 
-	/**
-	 * Callback used to sanitize the HTML provided by the user when generating previews of it in the editor.
-	 *
-	 * We strongly recommend overwriting the default function to avoid XSS vulnerabilities.
-	 *
-	 * Read more about the security aspect of this feature in the {@glink getting-started/setup/html-security "HTML security"}
-	 * guide.
-	 *
-	 * The function receives the input HTML (as a string), and should return an object
-	 * that matches the {@link module:core/editor/editorconfig~SanitizedOutput} interface.
-	 *
-	 * ```ts
-	 * ClassicEditor
-	 * 	.create( editorElement, {
-	 * 		sanitizeHtml( inputHtml ) {
-	 * 		// Strip unsafe elements and attributes, e.g.:
-	 * 		// the `<script>` elements and `on*` attributes.
-	 * 		const outputHtml = sanitize( inputHtml );
-	 *
-	 * 		return {
-	 * 			html: outputHtml,
-	 * 			// `true` or `false` depending on whether the sanitizer stripped anything.
-	 * 			hasChanged: inputHtml !== outputHtml
-	 * 		};
-	 *	} )
-	 * 	.then( ... )
-	 * 	.catch( ... );
-	 * ```
-	 *
-	 * This function is used by following features:
-	 *
-	 * * {@glink features/html/html-embed HTML embed}
-	 *   (when {@link module:html-embed/htmlembedconfig~HtmlEmbedConfig#showPreviews `showPreviews`} flag is set).
-	 * * {@glink features/merge-fields Merge fields}
-	 *   (when {@link module:merge-fields/mergefieldsconfig~MergeFieldsConfig#previewHtmlValues `previewHtmlValues`} flag is set).
-	 */
-	sanitizeHtml?: HtmlSanitizationCallback;
-
 	/**
 	 * Label text for the `aria-label` attribute set on editor editing area. Used by assistive technologies
 	 * to tell apart multiple editor instances (editing areas) on the page. If not set, a default
@@ -1062,22 +1024,3 @@ export interface UiConfig {
 	 **/
 	poweredBy?: PoweredByConfig;
 }
-
-/**
- * An object returned by the {@link module:core/editor/editorconfig~EditorConfig#sanitizeHtml} function.
- */
-export interface SanitizedOutput {
-
-	/**
-	 * An output (safe) HTML that will be inserted into the {@glink framework/architecture/editing-engine editing view}.
-	 */
-	html: string;
-
-	/**
-	 * A flag that indicates whether the output HTML is different than the input value.
-	 */
-	hasChanged: boolean;
-}
-
-// Workaround for the issue with TypeDoc not recognizing the `( html: string ) => SanitizedOutput` as a proper callable type.
-export type HtmlSanitizationCallback = ( html: string ) => SanitizedOutput;

packages/ckeditor5-core/src/index.ts

@@ -25,8 +25,7 @@ export type {
 	LanguageConfig,
 	ToolbarConfig,
 	ToolbarConfigItem,
-	UiConfig,
-	SanitizedOutput
+	UiConfig
 } from './editor/editorconfig.js';
 
 export { default as attachToForm } from './editor/utils/attachtoform.js';

packages/ckeditor5-core/tests/editor/editor.js

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
-/* globals document, window, setTimeout, console */
+/* globals document, window, setTimeout */
 
 import Editor from '../../src/editor/editor.js';
 import Context from '../../src/context.js';
@@ -218,28 +218,11 @@ describe( 'Editor', () => {
 			sinon.assert.calledWith( spy, editor.editing.view.document );
 		} );
 
-		describe( 'should define a pass-by sanitizing function which', () => {
-			let sanitizeHtml, editor;
-
-			beforeEach( () => {
-				sinon.stub( console, 'warn' );
-				editor = new TestEditor();
-				sanitizeHtml = editor.config.get( 'sanitizeHtml' );
-			} );
-
-			it( 'should return an input string (without any modifications)', () => {
-				expect( sanitizeHtml( 'foo' ) ).to.deep.equal( {
-					html: 'foo',
-					hasChanged: false
-				} );
-			} );
-
-			it( 'should display a warning when using the default sanitizer', () => {
-				sanitizeHtml( 'foo' );
-
-				expect( console.warn.callCount ).to.equal( 1 );
-				expect( console.warn.firstCall.args[ 0 ] ).to.equal( 'provide-sanitize-function' );
-			} );
+		it( 'should throw if `config.sanitizeHtml` is passed', () => {
+			expectToThrowCKEditorError( () => {
+				// eslint-disable-next-line no-new
+				new TestEditor( { sanitizeHtml: () => {} } );
+			}, 'editor-config-sanitizehtml-not-supported' );
 		} );
 	} );