Let opener service validate that only specific commands can be run in command uris (microsoft#165204)

Author: mjbvz

src/vs/editor/browser/services/openerService.ts

@@ -25,15 +25,25 @@ class CommandOpener implements IOpener {
 		if (!matchesScheme(target, Schemas.command)) {
 			return false;
 		}
+
 		if (!options?.allowCommands) {
 			// silently ignore commands when command-links are disabled, also
-			// surpress other openers by returning TRUE
+			// suppress other openers by returning TRUE
 			return true;
 		}
-		// run command or bail out if command isn't known
+
 		if (typeof target === 'string') {
 			target = URI.parse(target);
 		}
+
+		if (Array.isArray(options.allowCommands)) {
+			// Only allow specific commands
+			if (!options.allowCommands.includes(target.path)) {
+				// Suppress other openers by returning TRUE
+				return true;
+			}
+		}
+
 		// execute as command
 		let args: any = [];
 		try {

src/vs/editor/contrib/markdownRenderer/browser/markdownRenderer.ts

@@ -8,8 +8,6 @@ import { onUnexpectedError } from 'vs/base/common/errors';
 import { Emitter } from 'vs/base/common/event';
 import { IMarkdownString, MarkdownStringTrustedOptions } from 'vs/base/common/htmlContent';
 import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle';
-import { Schemas } from 'vs/base/common/network';
-import { URI } from 'vs/base/common/uri';
 import { applyFontInfo } from 'vs/editor/browser/config/domFontInfo';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { EditorOption } from 'vs/editor/common/config/editorOptions';
@@ -112,35 +110,26 @@ export class MarkdownRenderer {
 }
 
 export async function openLinkFromMarkdown(openerService: IOpenerService, link: string, isTrusted: boolean | MarkdownStringTrustedOptions | undefined): Promise<boolean> {
-	let allowCommands = false;
-	if (isTrusted) {
-		try {
-			const uri = URI.parse(link);
-			if (uri.scheme === Schemas.command) {
-				if (typeof isTrusted === 'boolean') {
-					if (!isTrusted) {
-						return false;
-					}
-
-					allowCommands = true;
-				} else {
-					// Only allow a subset of commands
-					if (!isTrusted.enabledCommands.includes(uri.path)) {
-						return false;
-					}
-
-					allowCommands = true;
-				}
-			}
-		} catch {
-			// noop
-		}
-	}
-
 	try {
-		return await openerService.open(link, { fromUserGesture: true, allowContributedOpeners: true, allowCommands });
+		return await openerService.open(link, {
+			fromUserGesture: true,
+			allowContributedOpeners: true,
+			allowCommands: toAllowCommandsOption(isTrusted),
+		});
 	} catch (e) {
 		onUnexpectedError(e);
 		return false;
 	}
 }
+
+function toAllowCommandsOption(isTrusted: boolean | MarkdownStringTrustedOptions | undefined): boolean | readonly string[] {
+	if (isTrusted === true) {
+		return true; // Allow all commands
+	}
+
+	if (isTrusted && Array.isArray(isTrusted.enabledCommands)) {
+		return isTrusted.enabledCommands; // Allow subset of commands
+	}
+
+	return false; // Block commands
+}

src/vs/platform/opener/common/opener.ts

@@ -33,8 +33,10 @@ export type OpenInternalOptions = {
 
 	/**
 	 * Allow command links to be handled.
+	 *
+	 * If this is an array, then only the commands included in the array can be run.
 	 */
-	readonly allowCommands?: boolean;
+	readonly allowCommands?: boolean | readonly string[];
 };
 
 export type OpenExternalOptions = {

src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts

@@ -682,35 +682,36 @@ var requirejs = (function() {
 					break;
 				}
 				case 'clicked-link': {
-					let linkToOpen: URI | string | undefined;
-
 					if (matchesScheme(data.href, Schemas.command)) {
-						// We allow a very limited set of commands
 						const uri = URI.parse(data.href);
-						switch (uri.path) {
-							case 'workbench.action.openLargeOutput': {
-								const outputId = uri.query;
-								const group = this.editorGroupService.activeGroup;
-								if (group) {
-									if (group.activeEditor) {
-										group.pinEditor(group.activeEditor);
-									}
-								}
 
-								this.openerService.open(CellUri.generateCellOutputUri(this.documentUri, outputId));
-								return;
-							}
-							case 'github-issues.authNow':
-							case 'workbench.extensions.search':
-							case 'workbench.action.openSettings': {
-								this.openerService.open(data.href, { fromUserGesture: true, allowCommands: true, fromWorkspace: true });
-								return;
+						if (uri.path === 'workbench.action.openLargeOutput') {
+							const outputId = uri.query;
+							const group = this.editorGroupService.activeGroup;
+							if (group) {
+								if (group.activeEditor) {
+									group.pinEditor(group.activeEditor);
+								}
 							}
+
+							this.openerService.open(CellUri.generateCellOutputUri(this.documentUri, outputId));
+							return;
 						}
 
+						// We allow a very limited set of commands
+						this.openerService.open(data.href, {
+							fromUserGesture: true,
+							fromWorkspace: true,
+							allowCommands: [
+								'github-issues.authNow',
+								'workbench.extensions.search',
+								'workbench.action.openSettings',
+							],
+						});
 						return;
 					}
 
+					let linkToOpen: URI | string | undefined;
 					if (matchesSomeScheme(data.href, Schemas.http, Schemas.https, Schemas.mailto, Schemas.vscodeNotebookCell, Schemas.vscodeNotebook)) {
 						linkToOpen = data.href;
 					} else if (!/^[\w\-]+:/.test(data.href)) {
@@ -742,7 +743,7 @@ var requirejs = (function() {
 					}
 
 					if (linkToOpen) {
-						this.openerService.open(linkToOpen, { fromUserGesture: true, allowCommands: false, fromWorkspace: true });
+						this.openerService.open(linkToOpen, { fromUserGesture: true, fromWorkspace: true });
 					}
 					break;
 				}