Check for allowed names in the participant fullName (#212849)

* Check for allowed names in the participant fullName

* Validate provider name and fullName

* Fix

src/vs/base/common/strings.ts

@@ -1249,6 +1249,16 @@ export class AmbiguousCharacters {
 		return this.confusableDictionary.has(codePoint);
 	}
 
+	public containsAmbiguousCharacter(str: string): boolean {
+		for (let i = 0; i < str.length; i++) {
+			const codePoint = str.codePointAt(i);
+			if (typeof codePoint === 'number' && this.isAmbiguous(codePoint)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	/**
 	 * Returns the non basic ASCII code point that the given code point can be confused,
 	 * or undefined if such code point does note exist.
@@ -1281,6 +1291,17 @@ export class InvisibleCharacters {
 		return InvisibleCharacters.getData().has(codePoint);
 	}
 
+	public static containsInvisibleCharacter(str: string): boolean {
+		for (let i = 0; i < str.length; i++) {
+			const codePoint = str.codePointAt(i);
+			if (typeof codePoint === 'number' && InvisibleCharacters.isInvisibleCharacter(codePoint)) {
+				return true;
+			}
+		}
+		return false;
+
+	}
+
 	public static get codePoints(): ReadonlySet<number> {
 		return InvisibleCharacters.getData();
 	}

src/vs/base/test/common/strings.test.ts

@@ -558,6 +558,22 @@ suite('Strings', () => {
 		assert.strictEqual(strings.count('hello world', 'foo'), 0);
 	});
 
+	test('containsAmbiguousCharacter', () => {
+		assert.strictEqual(strings.AmbiguousCharacters.getInstance(new Set()).containsAmbiguousCharacter('abcd'), false);
+		assert.strictEqual(strings.AmbiguousCharacters.getInstance(new Set()).containsAmbiguousCharacter('üå'), false);
+		assert.strictEqual(strings.AmbiguousCharacters.getInstance(new Set()).containsAmbiguousCharacter('(*&^)'), false);
+
+		assert.strictEqual(strings.AmbiguousCharacters.getInstance(new Set()).containsAmbiguousCharacter('ο'), true);
+		assert.strictEqual(strings.AmbiguousCharacters.getInstance(new Set()).containsAmbiguousCharacter('abɡc'), true);
+	});
+
+	test('containsInvisibleCharacter', () => {
+		assert.strictEqual(strings.InvisibleCharacters.containsInvisibleCharacter('abcd'), false);
+		assert.strictEqual(strings.InvisibleCharacters.containsInvisibleCharacter(' '), true);
+		assert.strictEqual(strings.InvisibleCharacters.containsInvisibleCharacter('a\u{e004e}b'), true);
+		assert.strictEqual(strings.InvisibleCharacters.containsInvisibleCharacter('a\u{e015a}\u000bb'), true);
+	});
+
 	ensureNoDisposablesAreLeakedInTestSuite();
 });
 

src/vs/workbench/contrib/chat/browser/chatAgentHover.ts

@@ -15,7 +15,7 @@ import { ThemeIcon } from 'vs/base/common/themables';
 import { URI } from 'vs/base/common/uri';
 import { localize } from 'vs/nls';
 import { ICommandService } from 'vs/platform/commands/common/commands';
-import { IChatAgentData, IChatAgentNameService, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
+import { getFullyQualifiedId, IChatAgentData, IChatAgentNameService, IChatAgentService } from 'vs/workbench/contrib/chat/common/chatAgents';
 import { showExtensionsWithIdsCommandId } from 'vs/workbench/contrib/extensions/browser/extensionsActions';
 import { verifiedPublisherIcon } from 'vs/workbench/contrib/extensions/browser/extensionsIcons';
 import { IExtensionsWorkbenchService } from 'vs/workbench/contrib/extensions/common/extensions';
@@ -87,7 +87,8 @@ export class ChatAgentHover extends Disposable {
 
 		this.domNode.classList.toggle('noExtensionName', !!agent.isDynamic);
 
-		this.name.textContent = `@${agent.name}`;
+		const isAllowed = this.chatAgentNameService.getAgentNameRestriction(agent);
+		this.name.textContent = isAllowed ? `@${agent.name}` : getFullyQualifiedId(agent);
 		this.extensionName.textContent = agent.extensionDisplayName;
 		this.publisherName.textContent = agent.publisherDisplayName ?? agent.extensionPublisherId;
 
@@ -99,7 +100,6 @@ export class ChatAgentHover extends Disposable {
 		}
 
 		this.description.textContent = description;
-		const isAllowed = this.chatAgentNameService.getAgentNameRestriction(agent).get();
 		this.domNode.classList.toggle('allowedName', isAllowed);
 
 		this.domNode.classList.toggle('verifiedPublisher', false);

src/vs/workbench/contrib/chat/browser/chatMarkdownDecorationsRenderer.ts

@@ -40,7 +40,7 @@ export function agentToMarkdown(agent: IChatAgentData, isClickable: boolean, acc
 	const chatAgentNameService = accessor.get(IChatAgentNameService);
 	const chatAgentService = accessor.get(IChatAgentService);
 
-	const isAllowed = chatAgentNameService.getAgentNameRestriction(agent).get();
+	const isAllowed = chatAgentNameService.getAgentNameRestriction(agent);
 	let name = `${isAllowed ? agent.name : getFullyQualifiedId(agent)}`;
 	const isDupe = isAllowed && chatAgentService.getAgentsByName(agent.name).length > 1;
 	if (isDupe) {

src/vs/workbench/contrib/chat/browser/chatParticipantContributions.ts

@@ -4,6 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { isNonEmptyArray } from 'vs/base/common/arrays';
+import * as strings from 'vs/base/common/strings';
 import { Codicon } from 'vs/base/common/codicons';
 import { Disposable, DisposableMap, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
 import { localize, localize2 } from 'vs/nls';
@@ -159,6 +160,22 @@ export class ChatExtensionPointHandler implements IWorkbenchContribution {
 		chatParticipantExtensionPoint.setHandler((extensions, delta) => {
 			for (const extension of delta.added) {
 				for (const providerDescriptor of extension.value) {
+					if (!providerDescriptor.name.match(/^[\w0-9_-]+$/)) {
+						this.logService.error(`Extension '${extension.description.identifier.value}' CANNOT register participant with invalid name: ${providerDescriptor.name}. Name must match /^[\\w0-9_-]+$/.`);
+						continue;
+					}
+
+					if (providerDescriptor.fullName && strings.AmbiguousCharacters.getInstance(new Set()).containsAmbiguousCharacter(providerDescriptor.fullName)) {
+						this.logService.error(`Extension '${extension.description.identifier.value}' CANNOT register participant with fullName that contains ambiguous characters: ${providerDescriptor.fullName}.`);
+						continue;
+					}
+
+					// Spaces are allowed but considered "invisible"
+					if (providerDescriptor.fullName && strings.InvisibleCharacters.containsInvisibleCharacter(providerDescriptor.fullName.replace(/ /g, ''))) {
+						this.logService.error(`Extension '${extension.description.identifier.value}' CANNOT register participant with fullName that contains invisible characters: ${providerDescriptor.fullName}.`);
+						continue;
+					}
+
 					if (providerDescriptor.isDefault && !isProposedApiEnabled(extension.description, 'defaultChatParticipant')) {
 						this.logService.error(`Extension '${extension.description.identifier.value}' CANNOT use API proposal: defaultChatParticipant.`);
 						continue;

src/vs/workbench/contrib/chat/browser/contrib/chatInputCompletions.ts

@@ -393,7 +393,7 @@ class VariableCompletions extends Disposable {
 Registry.as<IWorkbenchContributionsRegistry>(WorkbenchExtensions.Workbench).registerWorkbenchContribution(VariableCompletions, LifecyclePhase.Eventually);
 
 function getAgentCompletionDetails(agent: IChatAgentData, otherAgents: IChatAgentData[], chatAgentNameService: IChatAgentNameService): { label: string; isDupe: boolean } {
-	const isAllowed = chatAgentNameService.getAgentNameRestriction(agent).get();
+	const isAllowed = chatAgentNameService.getAgentNameRestriction(agent);
 	const agentLabel = `${chatAgentLeader}${isAllowed ? agent.name : getFullyQualifiedId(agent)}`;
 	const isDupe = isAllowed && !!otherAgents.find(other => other.name === agent.name && other.id !== agent.id);
 

src/vs/workbench/contrib/chat/common/chatAgents.ts

@@ -389,7 +389,7 @@ interface IChatParticipantRegistryResponse {
 
 export interface IChatAgentNameService {
 	_serviceBrand: undefined;
-	getAgentNameRestriction(chatAgentData: IChatAgentData): IObservable<boolean>;
+	getAgentNameRestriction(chatAgentData: IChatAgentData): boolean;
 }
 
 export class ChatAgentNameService implements IChatAgentNameService {
@@ -454,10 +454,20 @@ export class ChatAgentNameService implements IChatAgentNameService {
 		this.storageService.store(ChatAgentNameService.StorageKey, JSON.stringify(registry), StorageScope.APPLICATION, StorageTarget.MACHINE);
 	}
 
-	getAgentNameRestriction(chatAgentData: IChatAgentData): IObservable<boolean> {
+	/**
+	 * Returns true if the agent is allowed to use this name
+	 */
+	getAgentNameRestriction(chatAgentData: IChatAgentData): boolean {
+		// TODO would like to use observables here but nothing uses it downstream and I'm not sure how to combine these two
+		const nameAllowed = this.checkAgentNameRestriction(chatAgentData.name, chatAgentData).get();
+		const fullNameAllowed = !chatAgentData.fullName || this.checkAgentNameRestriction(chatAgentData.fullName.replace(/\s/g, ''), chatAgentData).get();
+		return nameAllowed && fullNameAllowed;
+	}
+
+	private checkAgentNameRestriction(name: string, chatAgentData: IChatAgentData): IObservable<boolean> {
 		// Registry is a map of name to an array of extension publisher IDs or extension IDs that are allowed to use it.
 		// Look up the list of extensions that are allowed to use this name
-		const allowList = this.registry.map<string[] | undefined>(registry => registry[chatAgentData.name.toLowerCase()]);
+		const allowList = this.registry.map<string[] | undefined>(registry => registry[name.toLowerCase()]);
 		return allowList.map(allowList => {
 			if (!allowList) {
 				return true;

src/vs/workbench/contrib/chat/common/chatViewModel.ts

@@ -4,6 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { Emitter, Event } from 'vs/base/common/event';
+import { IMarkdownString } from 'vs/base/common/htmlContent';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { marked } from 'vs/base/common/marked/marked';
 import { ThemeIcon } from 'vs/base/common/themables';
@@ -11,13 +12,12 @@ import { URI } from 'vs/base/common/uri';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { ILogService } from 'vs/platform/log/common/log';
 import { annotateVulnerabilitiesInText } from 'vs/workbench/contrib/chat/common/annotations';
-import { IChatAgentCommand, IChatAgentData, IChatAgentResult } from 'vs/workbench/contrib/chat/common/chatAgents';
+import { getFullyQualifiedId, IChatAgentCommand, IChatAgentData, IChatAgentNameService, IChatAgentResult } from 'vs/workbench/contrib/chat/common/chatAgents';
 import { ChatModelInitState, IChatModel, IChatRequestModel, IChatResponseModel, IChatTextEditGroup, IChatWelcomeMessageContent, IResponse } from 'vs/workbench/contrib/chat/common/chatModel';
 import { IParsedChatRequest } from 'vs/workbench/contrib/chat/common/chatParserTypes';
-import { IChatCommandButton, IChatConfirmation, IChatContentReference, IChatFollowup, IChatProgressMessage, IChatResponseErrorDetails, IChatResponseProgressFileTreeData, IChatTask, IChatUsedContext, IChatWarningMessage, ChatAgentVoteDirection } from 'vs/workbench/contrib/chat/common/chatService';
+import { ChatAgentVoteDirection, IChatCommandButton, IChatConfirmation, IChatContentReference, IChatFollowup, IChatProgressMessage, IChatResponseErrorDetails, IChatResponseProgressFileTreeData, IChatTask, IChatUsedContext, IChatWarningMessage } from 'vs/workbench/contrib/chat/common/chatService';
 import { countWords } from 'vs/workbench/contrib/chat/common/chatWordCounter';
 import { CodeBlockModelCollection } from './codeBlockModelCollection';
-import { IMarkdownString } from 'vs/base/common/htmlContent';
 
 export function isRequestVM(item: unknown): item is IChatRequestViewModel {
 	return !!item && typeof item === 'object' && 'message' in item;
@@ -369,9 +369,16 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi
 	}
 
 	get username() {
-		return this.agent ?
-			(this.agent.fullName || this.agent.name) :
-			this._model.username;
+		if (this.agent) {
+			const isAllowed = this.chatAgentNameService.getAgentNameRestriction(this.agent);
+			if (isAllowed) {
+				return this.agent.fullName || this.agent.name;
+			} else {
+				return getFullyQualifiedId(this.agent);
+			}
+		}
+
+		return this._model.username;
 	}
 
 	get avatarIcon() {
@@ -471,6 +478,7 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi
 	constructor(
 		private readonly _model: IChatResponseModel,
 		@ILogService private readonly logService: ILogService,
+		@IChatAgentNameService private readonly chatAgentNameService: IChatAgentNameService,
 	) {
 		super();