Merge branch 'main' into outer-takin

Author: mjbvz

src/vs/base/browser/domSanitize.ts

@@ -153,6 +153,18 @@ function hookDomPurifyHrefAndSrcSanitizer(allowedLinkProtocols: readonly string[
 	return toDisposable(() => dompurify.removeHook('afterSanitizeAttributes'));
 }
 
+/**
+ * Predicate that checks if an attribute should be kept or removed.
+ *
+ * @returns A boolean indicating whether the attribute should be kept or a string with the sanitized value (which implicitly keeps the attribute)
+ */
+export type SanitizeAttributePredicate = (node: Element, data: { readonly attrName: string; readonly attrValue: string }) => boolean | string;
+
+export interface SanitizeAttributeRule {
+	readonly attributeName: string;
+	shouldKeep: SanitizeAttributePredicate;
+}
+
 export interface DomSanitizerConfig {
 	/**
 	 * Configured the allowed html tags.
@@ -166,8 +178,8 @@ export interface DomSanitizerConfig {
 	 * Configured the allowed html attributes.
 	 */
 	readonly allowedAttributes?: {
-		readonly override?: readonly string[];
-		readonly augment?: readonly string[];
+		readonly override?: ReadonlyArray<string | SanitizeAttributeRule>;
+		readonly augment?: ReadonlyArray<string | SanitizeAttributeRule>;
 	};
 
 	/**
@@ -190,11 +202,6 @@ export interface DomSanitizerConfig {
 	 * For example, <p><bad>"text"</bad></p> becomes <p>"<bad>text</bad>"</p>.
 	 */
 	readonly replaceWithPlaintext?: boolean;
-
-	// TODO: move these into more controlled api
-	readonly _do_not_use_hooks?: {
-		readonly uponSanitizeAttribute?: UponSanitizeAttributeCb;
-	};
 }
 
 const defaultDomPurifyConfig = Object.freeze({
@@ -230,16 +237,27 @@ export function sanitizeHtml(untrusted: string, config?: DomSanitizerConfig): Tr
 			}
 		}
 
+		let resolvedAttributes: Array<string | SanitizeAttributeRule> = [...defaultAllowedAttrs];
 		if (config?.allowedAttributes) {
 			if (config.allowedAttributes.override) {
-				resolvedConfig.ALLOWED_ATTR = [...config.allowedAttributes.override];
+				resolvedAttributes = [...config.allowedAttributes.override];
 			}
 
 			if (config.allowedAttributes.augment) {
-				resolvedConfig.ALLOWED_ATTR = [...(resolvedConfig.ALLOWED_ATTR ?? []), ...config.allowedAttributes.augment];
+				resolvedAttributes = [...resolvedAttributes, ...config.allowedAttributes.augment];
+			}
+		}
+
+		const allowedAttrNames = new Set(resolvedAttributes.map(attr => typeof attr === 'string' ? attr : attr.attributeName));
+		const allowedAttrPredicates = new Map<string, SanitizeAttributeRule>();
+		for (const attr of resolvedAttributes) {
+			if (typeof attr !== 'string') {
+				allowedAttrPredicates.set(attr.attributeName, attr);
 			}
 		}
 
+		resolvedConfig.ALLOWED_ATTR = Array.from(allowedAttrNames);
+
 		store.add(hookDomPurifyHrefAndSrcSanitizer(
 			config?.allowedLinkProtocols?.override ?? [Schemas.http, Schemas.https],
 			config?.allowedMediaProtocols?.override ?? [Schemas.http, Schemas.https]));
@@ -248,8 +266,21 @@ export function sanitizeHtml(untrusted: string, config?: DomSanitizerConfig): Tr
 			store.add(addDompurifyHook('uponSanitizeElement', replaceWithPlainTextHook));
 		}
 
-		if (config?._do_not_use_hooks?.uponSanitizeAttribute) {
-			store.add(addDompurifyHook('uponSanitizeAttribute', config._do_not_use_hooks.uponSanitizeAttribute));
+		if (allowedAttrPredicates.size) {
+			store.add(addDompurifyHook('uponSanitizeAttribute', (node, e) => {
+				const predicate = allowedAttrPredicates.get(e.attrName);
+				if (predicate) {
+					const result = predicate.shouldKeep(node, e);
+					if (typeof result === 'string') {
+						e.keepAttr = true;
+						e.attrValue = result;
+					} else {
+						e.keepAttr = result;
+					}
+				} else {
+					e.keepAttr = allowedAttrNames.has(e.attrName);
+				}
+			}));
 		}
 
 		return dompurify.sanitize(untrusted, {

src/vs/base/browser/markdownRenderer.ts

@@ -57,9 +57,8 @@ export interface MarkdownSanitizerConfig {
 		readonly override: readonly string[];
 	};
 	readonly allowedAttributes?: {
-		readonly override: readonly string[];
+		readonly override: ReadonlyArray<string | domSanitize.SanitizeAttributeRule>;
 	};
-	readonly customAttrSanitizer?: (attrName: string, attrValue: string) => boolean | string;
 	readonly allowedLinkSchemes?: {
 		readonly augment: readonly string[];
 	};
@@ -451,14 +450,12 @@ export const allowedMarkdownHtmlTags = Object.freeze([
 	'input', // Allow inputs for rendering checkboxes. Other types of inputs are removed and the inputs are always disabled
 ]);
 
-export const allowedMarkdownHtmlAttributes = [
+export const allowedMarkdownHtmlAttributes = Object.freeze<Array<string | domSanitize.SanitizeAttributeRule>>([
 	'align',
 	'autoplay',
 	'alt',
-	'checked',
 	'colspan',
 	'controls',
-	'disabled',
 	'draggable',
 	'height',
 	'href',
@@ -473,16 +470,42 @@ export const allowedMarkdownHtmlAttributes = [
 	'type',
 	'width',
 	'start',
+
+	// Input (For disabled inputs)
+	'checked',
+	'disabled',
 	'value',
 
 	// Custom markdown attributes
 	'data-code',
 	'data-href',
 
-	// These attributes are sanitized in the hooks
-	'style',
-	'class',
-];
+	// Only allow very specific styles
+	{
+		attributeName: 'style',
+		shouldKeep: (element, data) => {
+			if (element.tagName === 'SPAN') {
+				if (data.attrName === 'style') {
+					return /^(color\:(#[0-9a-fA-F]+|var\(--vscode(-[a-zA-Z0-9]+)+\));)?(background-color\:(#[0-9a-fA-F]+|var\(--vscode(-[a-zA-Z0-9]+)+\));)?(border-radius:[0-9]+px;)?$/.test(data.attrValue);
+				}
+			}
+			return false;
+		}
+	},
+
+	// Only allow codicons for classes
+	{
+		attributeName: 'class',
+		shouldKeep: (element, data) => {
+			if (element.tagName === 'SPAN') {
+				if (data.attrName === 'class') {
+					return /^codicon codicon-[a-z\-]+( codicon-modifier-[a-z\-]+)?$/.test(data.attrValue);
+				}
+			}
+			return false;
+		},
+	},
+]);
 
 function getDomSanitizerConfig(isTrusted: boolean | MarkdownStringTrustedOptions, options: MarkdownSanitizerConfig): domSanitize.DomSanitizerConfig {
 	const allowedLinkSchemes = [
@@ -530,45 +553,6 @@ function getDomSanitizerConfig(isTrusted: boolean | MarkdownStringTrustedOptions
 			]
 		},
 		replaceWithPlaintext: options.replaceWithPlaintext,
-		_do_not_use_hooks: {
-			uponSanitizeAttribute: (element, e) => {
-				if (options.customAttrSanitizer) {
-					const result = options.customAttrSanitizer(e.attrName, e.attrValue);
-					if (typeof result === 'string') {
-						if (result) {
-							e.attrValue = result;
-							e.keepAttr = true;
-						} else {
-							e.keepAttr = false;
-						}
-					} else {
-						e.keepAttr = result;
-					}
-					return;
-				}
-
-				if (e.attrName === 'style' || e.attrName === 'class') {
-					if (element.tagName === 'SPAN') {
-						if (e.attrName === 'style') {
-							e.keepAttr = /^(color\:(#[0-9a-fA-F]+|var\(--vscode(-[a-zA-Z0-9]+)+\));)?(background-color\:(#[0-9a-fA-F]+|var\(--vscode(-[a-zA-Z0-9]+)+\));)?(border-radius:[0-9]+px;)?$/.test(e.attrValue);
-							return;
-						} else if (e.attrName === 'class') {
-							e.keepAttr = /^codicon codicon-[a-z\-]+( codicon-modifier-[a-z\-]+)?$/.test(e.attrValue);
-							return;
-						}
-					}
-
-					e.keepAttr = false;
-					return;
-				} else if (element.tagName === 'INPUT' && element.attributes.getNamedItem('type')?.value === 'checkbox') {
-					if ((e.attrName === 'type' && e.attrValue === 'checkbox') || e.attrName === 'disabled' || e.attrName === 'checked') {
-						e.keepAttr = true;
-						return;
-					}
-					e.keepAttr = false;
-				}
-			},
-		}
 	};
 }
 

src/vs/base/test/browser/domSanitize.test.ts

@@ -3,10 +3,10 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { ensureNoDisposablesAreLeakedInTestSuite } from '../common/utils.js';
-import { sanitizeHtml } from '../../browser/domSanitize.js';
 import * as assert from 'assert';
+import { sanitizeHtml } from '../../browser/domSanitize.js';
 import { Schemas } from '../../common/network.js';
+import { ensureNoDisposablesAreLeakedInTestSuite } from '../common/utils.js';
 
 suite('DomSanitize', () => {
 
@@ -114,6 +114,45 @@ suite('DomSanitize', () => {
 		assert.ok(str.includes('src="data:image/png;base64,'));
 	});
 
+	test('Supports dynamic attribute sanitization', () => {
+		const html = '<div title="a" other="1">text1</div><div title="b" other="2">text2</div>';
+		const result = sanitizeHtml(html, {
+			allowedAttributes: {
+				override: [
+					{
+						attributeName: 'title',
+						shouldKeep: (_el, data) => {
+							return data.attrValue.includes('b');
+						}
+					}
+				]
+			}
+		});
+		const str = result.toString();
+		assert.strictEqual(str, '<div>text1</div><div title="b">text2</div>');
+	});
+
+	test('Supports changing attributes in dynamic sanitization', () => {
+		const html = '<div title="abc" other="1">text1</div><div title="xyz" other="2">text2</div>';
+		const result = sanitizeHtml(html, {
+			allowedAttributes: {
+				override: [
+					{
+						attributeName: 'title',
+						shouldKeep: (_el, data) => {
+							if (data.attrValue === 'abc') {
+								return false;
+							}
+							return data.attrValue + data.attrValue;
+						}
+					}
+				]
+			}
+		});
+		// xyz title should be preserved and doubled
+		assert.strictEqual(result.toString(), '<div>text1</div><div title="xyzxyz">text2</div>');
+	});
+
 	suite('replaceWithPlaintext', () => {
 
 		test('replaces unsupported tags with plaintext representation', () => {

src/vs/platform/debug/common/extensionHostDebug.ts

@@ -48,4 +48,6 @@ export interface IExtensionHostDebugService {
 	readonly onTerminateSession: Event<ITerminateSessionEvent>;
 
 	openExtensionDevelopmentHostWindow(args: string[], debugRenderer: boolean): Promise<IOpenExtensionWindowResult>;
+
+	attachToCurrentWindowRenderer(windowId: number): Promise<IOpenExtensionWindowResult>;
 }

src/vs/platform/debug/common/extensionHostDebugIpc.ts

@@ -89,4 +89,8 @@ export class ExtensionHostDebugChannelClient extends Disposable implements IExte
 	openExtensionDevelopmentHostWindow(args: string[], debugRenderer: boolean): Promise<IOpenExtensionWindowResult> {
 		return this.channel.call('openExtensionDevelopmentHostWindow', [args, debugRenderer]);
 	}
+
+	attachToCurrentWindowRenderer(windowId: number): Promise<IOpenExtensionWindowResult> {
+		return this.channel.call('attachToCurrentWindowRenderer', [windowId]);
+	}
 }

src/vs/platform/debug/electron-main/extensionHostDebugIpc.ts

@@ -3,11 +3,12 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { BrowserWindow } from 'electron';
 import { AddressInfo, createServer } from 'net';
-import { IOpenExtensionWindowResult } from '../common/extensionHostDebug.js';
-import { ExtensionHostDebugBroadcastChannel } from '../common/extensionHostDebugIpc.js';
 import { OPTIONS, parseArgs } from '../../environment/node/argv.js';
 import { IWindowsMainService, OpenContext } from '../../windows/electron-main/windows.js';
+import { IOpenExtensionWindowResult } from '../common/extensionHostDebug.js';
+import { ExtensionHostDebugBroadcastChannel } from '../common/extensionHostDebugIpc.js';
 
 export class ElectronExtensionHostDebugBroadcastChannel<TContext> extends ExtensionHostDebugBroadcastChannel<TContext> {
 
@@ -20,11 +21,22 @@ export class ElectronExtensionHostDebugBroadcastChannel<TContext> extends Extens
 	override call(ctx: TContext, command: string, arg?: any): Promise<any> {
 		if (command === 'openExtensionDevelopmentHostWindow') {
 			return this.openExtensionDevelopmentHostWindow(arg[0], arg[1]);
+		} else if (command === 'attachToCurrentWindowRenderer') {
+			return this.attachToCurrentWindowRenderer(arg[0]);
 		} else {
 			return super.call(ctx, command, arg);
 		}
 	}
 
+	private async attachToCurrentWindowRenderer(windowId: number): Promise<IOpenExtensionWindowResult> {
+		const codeWindow = this.windowsMainService.getWindowById(windowId);
+		if (!codeWindow?.win) {
+			return { success: false };
+		}
+
+		return this.openCdp(codeWindow.win);
+	}
+
 	private async openExtensionDevelopmentHostWindow(args: string[], debugRenderer: boolean): Promise<IOpenExtensionWindowResult> {
 		const pargs = parseArgs(args, OPTIONS);
 		pargs.debugRenderer = debugRenderer;
@@ -50,6 +62,10 @@ export class ElectronExtensionHostDebugBroadcastChannel<TContext> extends Extens
 			return { success: true };
 		}
 
+		return this.openCdp(win);
+	}
+
+	private async openCdp(win: BrowserWindow): Promise<IOpenExtensionWindowResult> {
 		const debug = win.webContents.debugger;
 
 		let listeners = debug.isAttached() ? Infinity : 0;

src/vs/workbench/api/common/extHostChatSessions.ts

@@ -75,7 +75,7 @@ export class ExtHostChatSessions extends Disposable implements ExtHostChatSessio
 		commands.registerArgumentProcessor({
 			processArgument: (arg) => {
 				if (arg && arg.$mid === MarshalledId.ChatSessionContext) {
-					const id = arg.id;
+					const id = arg.session.id;
 					const sessionContent = this._sessionMap.get(id);
 					if (sessionContent) {
 						return sessionContent;

src/vs/workbench/contrib/chat/browser/actions/chatActions.ts

@@ -580,7 +580,7 @@ export function registerChatActions() {
 
 								const ckey = contextKeyService.createKey('chatSessionType', session.providerType);
 								const actions = menuService.getMenuActions(MenuId.ChatSessionsMenu, contextKeyService);
-								const menuActions = getContextMenuActions(actions, 'navigation');
+								const menuActions = getContextMenuActions(actions, 'inline');
 								ckey.reset();
 
 								// Use primary actions if available, otherwise fall back to secondary actions
@@ -756,7 +756,7 @@ export function registerChatActions() {
 						const contextItem = context.item as ICodingAgentPickerItem;
 						commandService.executeCommand(buttonItem.id, {
 							uri: contextItem.uri,
-							session: contextItem.session,
+							session: contextItem.session?.session,
 							$mid: MarshalledId.ChatSessionContext
 						});
 

src/vs/workbench/contrib/chat/browser/chatContentParts/chatConfirmationWidget.ts

@@ -41,7 +41,7 @@ export interface IChatConfirmationWidgetOptions {
 	title: string | IMarkdownString;
 	subtitle?: string | IMarkdownString;
 	buttons: IChatConfirmationButton[];
-	toolbarData?: { arg: any; partType: string };
+	toolbarData?: { arg: any; partType: string; partSource?: string };
 }
 
 export class ChatQueryTitlePart extends Disposable {
@@ -209,7 +209,10 @@ abstract class BaseChatConfirmationWidget extends Disposable {
 
 		// Create toolbar if actions are provided
 		if (options?.toolbarData) {
-			const overlay = contextKeyService.createOverlay([['chatConfirmationPartType', options.toolbarData.partType]]);
+			const overlay = contextKeyService.createOverlay([
+				['chatConfirmationPartType', options.toolbarData.partType],
+				['chatConfirmationPartSource', options.toolbarData.partSource],
+			]);
 			const nestedInsta = this._register(instantiationService.createChild(new ServiceCollection([IContextKeyService, overlay])));
 			this._register(nestedInsta.createInstance(
 				MenuWorkbenchToolBar,

src/vs/workbench/contrib/chat/browser/chatContentParts/chatElicitationContentPart.ts

@@ -31,7 +31,7 @@ export class ChatElicitationContentPart extends Disposable implements IChatConte
 			{ label: elicitation.acceptButtonLabel, data: true },
 			{ label: elicitation.rejectButtonLabel, data: false, isSecondary: true },
 		];
-		const confirmationWidget = this._register(this.instantiationService.createInstance(ChatConfirmationWidget, context.container, { title: elicitation.title, subtitle: elicitation.subtitle, buttons, message: this.getMessageToRender(elicitation), toolbarData: { partType: elicitation.source ? `${elicitation.source.type}Elicitation` : 'elicitation', arg: elicitation } }));
+		const confirmationWidget = this._register(this.instantiationService.createInstance(ChatConfirmationWidget, context.container, { title: elicitation.title, subtitle: elicitation.subtitle, buttons, message: this.getMessageToRender(elicitation), toolbarData: { partType: 'elicitation', partSource: elicitation.source?.type, arg: elicitation } }));
 		confirmationWidget.setShowButtons(elicitation.state === 'pending');
 
 		this._register(elicitation.onDidRequestHide(() => this.domNode.remove()));