Merge pull request microsoft#172576 from dtivel/dtivel/do-not-block

Do not block if unable to verify extension signature

Author: sandy081

src/vs/platform/extensionManagement/node/extensionDownloader.ts

@@ -68,11 +68,14 @@ export class ExtensionsDownloader extends Disposable {
 				}
 				this.logService.info(`Extension signature verification: ${extension.identifier.id}. Verification status: ${verificationStatus}.`);
 			} catch (error) {
-				const code: string = (error as ExtensionSignatureVerificationError).code;
+				const sigError = error as ExtensionSignatureVerificationError;
+				const code: string = sigError.code;
 
 				if (code === 'UnknownError') {
 					verificationStatus = ExtensionVerificationStatus.UnknownError;
 					this.logService.warn(`Extension signature verification: ${extension.identifier.id}. Verification status: ${verificationStatus}.`);
+				} else if (!sigError.didExecute) {
+					this.logService.warn(`Extension signature verification: ${extension.identifier.id}. Verification status: ${verificationStatus} (${code})`);
 				} else {
 					await this.delete(signatureArchiveLocation);
 					await this.delete(location);

src/vs/platform/extensionManagement/node/extensionSignatureVerificationService.ts

@@ -34,6 +34,7 @@ declare module vsceSign {
  */
 export interface ExtensionSignatureVerificationError extends Error {
 	readonly code: string;
+	readonly didExecute: boolean;
 }
 
 export class ExtensionSignatureVerificationService implements IExtensionSignatureVerificationService {

src/vs/platform/extensionManagement/test/node/installGalleryExtensionTask.test.ts

@@ -37,7 +37,9 @@ class TestExtensionsScanner extends mock<ExtensionsScanner>() {
 
 class TestExtensionSignatureVerificationService extends mock<IExtensionSignatureVerificationService>() {
 
-	constructor(private readonly verificationResult: string | boolean) {
+	constructor(
+		private readonly verificationResult: string | boolean,
+		private readonly didExecute: boolean) {
 		super();
 	}
 
@@ -47,6 +49,7 @@ class TestExtensionSignatureVerificationService extends mock<IExtensionSignature
 		}
 		const error = Error(this.verificationResult);
 		(error as any).code = this.verificationResult;
+		(error as any).didExecute = this.didExecute;
 		throw error;
 	}
 }
@@ -89,7 +92,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
 	teardown(() => disposables.clear());
 
 	test('if verification is enabled by default, the task completes', async () => {
-		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader(true));
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: true }));
 
 		await testObject.run();
 
@@ -98,7 +101,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
 	});
 
 	test('if verification is disabled in stable, the task completes', async () => {
-		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader('error', undefined, 'stable'));
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: false, verificationResult: 'error', didExecute: true, quality: 'stable' }));
 
 		await testObject.run();
 
@@ -107,7 +110,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
 	});
 
 	test('if verification is disabled by setting set to false, the task skips verification', async () => {
-		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader('error', false));
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: false, verificationResult: 'error', didExecute: false }));
 
 		await testObject.run();
 
@@ -116,7 +119,17 @@ suite('InstallGalleryExtensionTask Tests', () => {
 	});
 
 	test('if verification is disabled because the module is not loaded, the task skips verification', async () => {
-		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader(false, true));
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: false, didExecute: false }));
+
+		await testObject.run();
+
+		assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Unverified);
+		assert.strictEqual(testObject.installed, true);
+	});
+
+	test('if verification fails to execute, the task completes', async () => {
+		const errorCode = 'ENOENT';
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode, didExecute: false }));
 
 		await testObject.run();
 
@@ -127,7 +140,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
 	test('if verification fails, the task throws', async () => {
 		const errorCode = 'IntegrityCheckFailed';
 
-		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader(errorCode, true));
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: errorCode, didExecute: true }));
 
 		try {
 			await testObject.run();
@@ -141,11 +154,10 @@ suite('InstallGalleryExtensionTask Tests', () => {
 		}
 
 		assert.fail('It should have thrown.');
-
 	});
 
 	test('if verification succeeds, the task completes', async () => {
-		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader(true, true));
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: true }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: true }));
 
 		await testObject.run();
 
@@ -154,7 +166,7 @@ suite('InstallGalleryExtensionTask Tests', () => {
 	});
 
 	test('task completes for unsigned extension', async () => {
-		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader(true, true));
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: true, didExecute: false }));
 
 		await testObject.run();
 
@@ -163,22 +175,22 @@ suite('InstallGalleryExtensionTask Tests', () => {
 	});
 
 	test('task completes for an unsigned extension even when signature verification throws error', async () => {
-		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader('error', true));
+		const testObject = new TestInstallGalleryExtensionTask(aGalleryExtension('a', { isSigned: false }), anExtensionsDownloader({ isSignatureVerificationEnabled: true, verificationResult: 'error', didExecute: true }));
 
 		await testObject.run();
 
 		assert.strictEqual(testObject.verificationStatus, ExtensionVerificationStatus.Unverified);
 		assert.strictEqual(testObject.installed, true);
 	});
 
-	function anExtensionsDownloader(verificationResult: string | boolean, isSignatureVerificationEnabled?: boolean, quality?: string): ExtensionsDownloader {
+	function anExtensionsDownloader(options: { isSignatureVerificationEnabled: boolean; verificationResult: boolean | string; didExecute: boolean; quality?: string }): ExtensionsDownloader {
 		const logService = new NullLogService();
 		const fileService = disposables.add(new FileService(logService));
 		const fileSystemProvider = disposables.add(new InMemoryFileSystemProvider());
 		fileService.registerProvider(ROOT.scheme, fileSystemProvider);
 
 		const instantiationService = new TestInstantiationService();
-		instantiationService.stub(IProductService, { quality: quality ?? 'insiders' });
+		instantiationService.stub(IProductService, { quality: options.quality ?? 'insiders' });
 		instantiationService.stub(IFileService, fileService);
 		instantiationService.stub(ILogService, logService);
 		instantiationService.stub(INativeEnvironmentService, <Partial<INativeEnvironmentService>>{ extensionsDownloadLocation: joinPath(ROOT, 'CachedExtensionVSIXs') });
@@ -190,8 +202,8 @@ suite('InstallGalleryExtensionTask Tests', () => {
 				await fileService.writeFile(location, VSBuffer.fromString('extension signature'));
 			},
 		});
-		instantiationService.stub(IConfigurationService, new TestConfigurationService(isBoolean(isSignatureVerificationEnabled) ? { extensions: { verifySignature: isSignatureVerificationEnabled } } : undefined));
-		instantiationService.stub(IExtensionSignatureVerificationService, new TestExtensionSignatureVerificationService(verificationResult));
+		instantiationService.stub(IConfigurationService, new TestConfigurationService(isBoolean(options.isSignatureVerificationEnabled) ? { extensions: { verifySignature: options.isSignatureVerificationEnabled } } : undefined));
+		instantiationService.stub(IExtensionSignatureVerificationService, new TestExtensionSignatureVerificationService(options.verificationResult, !!options.didExecute));
 		return instantiationService.createInstance(ExtensionsDownloader);
 	}