Skip to content

Small additional cleanups to domSanitize #256579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 17, 2025
Merged

Small additional cleanups to domSanitize #256579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/vs/base/browser/domSanitize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ export interface SanitizeOptions {
readonly override?: readonly string[];
};

readonly hooks?: {
// TODO: move these into more controlled api
readonly _do_not_use_hooks?: {
readonly uponSanitizeElement?: UponSanitizeElementCb;
readonly uponSanitizeAttribute?: UponSanitizeAttributeCb;
};
Expand Down Expand Up @@ -218,7 +219,7 @@ export function sanitizeHtml(untrusted: string, config?: SanitizeOptions): Trust
resolvedConfig.ALLOWED_TAGS = [...config.allowedTags.override];
}

if (config?.allowedTags?.augment) {
if (config.allowedTags.augment) {
resolvedConfig.ALLOWED_TAGS = [...(resolvedConfig.ALLOWED_TAGS ?? []), ...config.allowedTags.augment];
}
}
Expand All @@ -228,7 +229,7 @@ export function sanitizeHtml(untrusted: string, config?: SanitizeOptions): Trust
resolvedConfig.ALLOWED_ATTR = [...config.allowedAttributes.override];
}

if (config?.allowedAttributes?.augment) {
if (config.allowedAttributes.augment) {
resolvedConfig.ALLOWED_ATTR = [...(resolvedConfig.ALLOWED_ATTR ?? []), ...config.allowedAttributes.augment];
}
}
Expand All @@ -237,12 +238,12 @@ export function sanitizeHtml(untrusted: string, config?: SanitizeOptions): Trust
config?.allowedLinkProtocols?.override ?? [Schemas.http, Schemas.https],
config?.allowedMediaProtocols?.override ?? [Schemas.http, Schemas.https]));

if (config?.hooks?.uponSanitizeElement) {
store.add(addDompurifyHook('uponSanitizeElement', config?.hooks.uponSanitizeElement));
if (config?._do_not_use_hooks?.uponSanitizeElement) {
store.add(addDompurifyHook('uponSanitizeElement', config?._do_not_use_hooks.uponSanitizeElement));
}

if (config?.hooks?.uponSanitizeAttribute) {
store.add(addDompurifyHook('uponSanitizeAttribute', config.hooks.uponSanitizeAttribute));
if (config?._do_not_use_hooks?.uponSanitizeAttribute) {
store.add(addDompurifyHook('uponSanitizeAttribute', config._do_not_use_hooks.uponSanitizeAttribute));
}

return dompurify.sanitize(untrusted, {
Expand All @@ -257,6 +258,6 @@ export function sanitizeHtml(untrusted: string, config?: SanitizeOptions): Trust
/**
* Sanitizes the given `value` and reset the given `node` with it.
*/
export function safeInnerHtml(node: HTMLElement, untrusted: string, config?: SanitizeOptions): void {
export function safeSetInnerHtml(node: HTMLElement, untrusted: string, config?: SanitizeOptions): void {
node.innerHTML = sanitizeHtml(untrusted, config) as any;
}
8 changes: 5 additions & 3 deletions src/vs/base/browser/markdownRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ export interface MarkdownRenderOptions extends FormattedTextRenderOptions {

export interface ISanitizerOptions {
readonly replaceWithPlaintext?: boolean;
readonly allowedTags?: readonly string[];
readonly allowedTags?: {
readonly override: readonly string[];
};
readonly customAttrSanitizer?: (attrName: string, attrValue: string) => boolean | string;
readonly allowedProductProtocols?: readonly string[];
}
Expand Down Expand Up @@ -472,7 +474,7 @@ function getSanitizerOptions(options: IInternalSanitizerOptions): domSanitize.Sa
// HTML tags that can result from markdown are from reading https://spec.commonmark.org/0.29/
// HTML table tags that can result from markdown are from https://github.github.com/gfm/#tables-extension-
allowedTags: {
override: options.allowedTags ?? domSanitize.basicMarkupHtmlTags
override: options.allowedTags?.override ?? domSanitize.basicMarkupHtmlTags
},
allowedAttributes: {
override: allowedMarkdownHtmlAttributes,
Expand All @@ -491,7 +493,7 @@ function getSanitizerOptions(options: IInternalSanitizerOptions): domSanitize.Sa
Schemas.vscodeRemoteResource,
]
},
hooks: {
_do_not_use_hooks: {
uponSanitizeAttribute: (element, e) => {
if (options.customAttrSanitizer) {
const result = options.customAttrSanitizer(e.attrName, e.attrValue);
Expand Down
6 changes: 3 additions & 3 deletions src/vs/base/browser/ui/button/button.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { localize } from '../../../../nls.js';
import type { IManagedHover } from '../hover/hover.js';
import { getBaseLayerHoverDelegate } from '../hover/hoverDelegate2.js';
import { IActionProvider } from '../dropdown/dropdown.js';
import { safeInnerHtml, SanitizeOptions } from '../../domSanitize.js';
import { safeSetInnerHtml, SanitizeOptions } from '../../domSanitize.js';

export interface IButtonOptions extends Partial<IButtonStyles> {
readonly title?: boolean | string;
Expand Down Expand Up @@ -253,7 +253,7 @@ export class Button extends Disposable implements IButton {
// Don't include outer `<p>`
const root = rendered.element.querySelector('p')?.innerHTML;
if (root) {
safeInnerHtml(labelElement, root, buttonSanitizerOptions);
safeSetInnerHtml(labelElement, root, buttonSanitizerOptions);
} else {
reset(labelElement);
}
Expand Down Expand Up @@ -654,7 +654,7 @@ export class ButtonWithIcon extends Button {

const root = rendered.element.querySelector('p')?.innerHTML;
if (root) {
safeInnerHtml(this._mdlabelElement, root, buttonSanitizerOptions);
safeSetInnerHtml(this._mdlabelElement, root, buttonSanitizerOptions);
} else {
reset(this._mdlabelElement);
}
Expand Down
4 changes: 3 additions & 1 deletion src/vs/workbench/contrib/chat/browser/chatMarkdownRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ export class ChatMarkdownRenderer extends MarkdownRenderer {
remoteImageIsAllowed: (_uri) => false,
sanitizerOptions: {
replaceWithPlaintext: true,
allowedTags: allowedChatMarkdownHtmlTags,
allowedTags: {
override: allowedChatMarkdownHtmlTags,
},
...options?.sanitizerOptions,
allowedProductProtocols: [product.urlProtocol]
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/issue/browser/issueFormService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { safeInnerHtml } from '../../../../base/browser/domSanitize.js';
import { safeSetInnerHtml } from '../../../../base/browser/domSanitize.js';
import { DisposableStore } from '../../../../base/common/lifecycle.js';
import { isLinux, isWindows } from '../../../../base/common/platform.js';
import Severity from '../../../../base/common/severity.js';
Expand Down Expand Up @@ -93,7 +93,7 @@ export class IssueFormService implements IIssueFormService {
// removes preset monaco-workbench
auxiliaryWindow.container.remove();
auxiliaryWindow.window.document.body.appendChild(div);
safeInnerHtml(div, BaseHtml(), {
safeSetInnerHtml(div, BaseHtml(), {
// Also allow input elements
allowedTags: {
augment: [
Expand Down
10 changes: 6 additions & 4 deletions src/vs/workbench/contrib/markdown/browser/markedKatexSupport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ export class MarkedKatexSupport {
readonly allowedAttributes: readonly string[];
}): ISanitizerOptions {
return {
allowedTags: [
...baseConfig.allowedTags,
...trustedMathMlTags,
],
allowedTags: {
override: [
...baseConfig.allowedTags,
...trustedMathMlTags,
]
},
customAttrSanitizer: (attrName, attrValue) => {
if (attrName === 'class') {
return true; // TODO: allows all classes for now since we don't have a list of possible katex classes
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ export class CodeCell extends Disposable {
if (this.viewCell.isInputCollapsed && this._inputCollapseElement) {
// flush the collapsed input with the latest tokens
const content = this._getRichTextFromLineTokens(model);
domSanitize.safeInnerHtml(this._inputCollapseElement, content);
domSanitize.safeSetInnerHtml(this._inputCollapseElement, content);
this._attachInputExpandButton(this._inputCollapseElement);
}
}));
Expand Down Expand Up @@ -442,7 +442,7 @@ export class CodeCell extends Disposable {
// update preview
const richEditorText = this.templateData.editor.hasModel() ? this._getRichTextFromLineTokens(this.templateData.editor.getModel()) : this._getRichText(this.viewCell.textBuffer, this.viewCell.language);
const element = DOM.$('div.cell-collapse-preview');
domSanitize.safeInnerHtml(element, richEditorText);
domSanitize.safeSetInnerHtml(element, richEditorText);
this._inputCollapseElement = element;
this.templateData.cellInputCollapsedContainer.appendChild(element);
this._attachInputExpandButton(element);
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/notebook/browser/view/cellParts/markupCell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export class MarkupCell extends Disposable {
const element = DOM.$('div');
element.classList.add('cell-collapse-preview');
const richEditorText = this.getRichText(this.viewCell.textBuffer, this.viewCell.language);
domSanitize.safeInnerHtml(element, richEditorText);
domSanitize.safeSetInnerHtml(element, richEditorText);
this.templateData.cellInputCollapsedContainer.appendChild(element);

const expandIcon = DOM.append(element, DOM.$('span.expandInputIcon'));
Expand Down Expand Up @@ -404,7 +404,7 @@ export class MarkupCell extends Disposable {
this.markdownAccessibilityContainer.innerText = '';
if (this.viewCell.renderedHtml) {
if (this.accessibilityService.isScreenReaderOptimized()) {
domSanitize.safeInnerHtml(this.markdownAccessibilityContainer, this.viewCell.renderedHtml);
domSanitize.safeSetInnerHtml(this.markdownAccessibilityContainer, this.viewCell.renderedHtml);
} else {
DOM.clearNode(this.markdownAccessibilityContainer);
}
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/welcomeWalkthrough/browser/walkThroughPart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ export class WalkThroughPart extends EditorPane {
}

private safeSetInnerHtml(node: HTMLElement, content: string) {
domSanitize.safeInnerHtml(node, content, {
domSanitize.safeSetInnerHtml(node, content, {
allowedAttributes: {
augment: [
'id',
Expand Down
Loading