Query security manager extension permissions concurrently (#400)

src/virtual-machine.js

@@ -772,15 +772,15 @@ class VirtualMachine extends EventEmitter {
      */
     async _loadExtensions (extensionIDs, extensionURLs = new Map()) {
         const defaultExtensionURLs = require('./extension-support/tw-default-extension-urls');
-        const extensionPromises = [];
+        const builtinExtensionIDs = [];
+        const customExtensionIDs = [];
+        const customExtensionURLs = [];
         for (const extensionID of extensionIDs) {
             if (this.extensionManager.isExtensionLoaded(extensionID)) {
                 // Already loaded
             } else if (this.extensionManager.isBuiltinExtension(extensionID)) {
-                // Builtin extension
-                this.extensionManager.loadExtensionIdSync(extensionID);
+                builtinExtensionIDs.push(extensionID);
             } else {
-                // Custom extension
                 let url = extensionURLs.get(extensionID);
                 if (!url && Object.prototype.hasOwnProperty.call(defaultExtensionURLs, extensionID)) {
                     url = defaultExtensionURLs[extensionID];
@@ -788,14 +788,25 @@ class VirtualMachine extends EventEmitter {
                 if (!url) {
                     throw new Error(`Unknown extension: ${extensionID}`);
                 }
-                if (await this.securityManager.canLoadExtensionFromProject(url)) {
-                    extensionPromises.push(this.extensionManager.loadExtensionURL(url));
-                } else {
-                    throw new Error(`Permission to load extension denied: ${extensionID}`);
-                }
+                customExtensionIDs.push(extensionID);
+                customExtensionURLs.push(url);
             }
         }
-        return Promise.all(extensionPromises);
+
+        // Ask about every custom extension concurrently, before loading any of them, so that the
+        // security manager can combine them into a single prompt if it wants to.
+        const permissions = await Promise.all(
+            customExtensionURLs.map(url => this.securityManager.canLoadExtensionFromProject(url))
+        );
+        const deniedExtensionIDs = customExtensionIDs.filter((_, i) => !permissions[i]);
+        if (deniedExtensionIDs.length > 0) {
+            throw new Error(`Permission to load extension denied: ${deniedExtensionIDs.join(', ')}`);
+        }
+
+        for (const extensionID of builtinExtensionIDs) {
+            this.extensionManager.loadExtensionIdSync(extensionID);
+        }
+        return Promise.all(customExtensionURLs.map(url => this.extensionManager.loadExtensionURL(url)));
     }
 
     /**

test/integration/tw_default_extension_url.js

@@ -92,9 +92,10 @@ test('Loading project uses default extension URLs', async () => {
         }
     });
     assert.deepEqual(events, [
+        // All permission checks happen before any extensions are loaded
         'canLoadExtensionFromProject https://extensions.turbowarp.org/lab/text.js',
-        'loadExtensionURL https://extensions.turbowarp.org/lab/text.js',
         'canLoadExtensionFromProject https://example.com/box2d.js',
+        'loadExtensionURL https://extensions.turbowarp.org/lab/text.js',
         'loadExtensionURL https://example.com/box2d.js'
     ]);
 });

test/integration/tw_security_manager.js

@@ -54,6 +54,36 @@ test('Allow both extensions', async () => {
     assert.deepEqual(new Set(loadedExtensions), new Set([FETCH_EXTENSION, BITWISE_EXTENSION]));
 });
 
+test('Asks about every extension concurrently before loading any', async () => {
+    const vm = new VirtualMachine();
+    const loadedExtensions = [];
+    vm.extensionManager.loadExtensionURL = url => {
+        loadedExtensions.push(url);
+        return Promise.resolve();
+    };
+    const pending = [];
+    let resolveAllAsked;
+    const allAsked = new Promise(resolve => {
+        resolveAllAsked = resolve;
+    });
+    vm.securityManager.canLoadExtensionFromProject = url => new Promise(resolve => {
+        pending.push({url, resolve});
+        if (pending.length === 2) {
+            resolveAllAsked();
+        }
+    });
+    const loadPromise = vm.loadProject(testProject);
+    // Neither request is answered until both have been made
+    await allAsked;
+    assert.deepEqual(new Set(pending.map(i => i.url)), new Set([FETCH_EXTENSION, BITWISE_EXTENSION]));
+    assert.deepEqual(loadedExtensions, []);
+    for (const {resolve} of pending) {
+        resolve(true);
+    }
+    await loadPromise;
+    assert.deepEqual(new Set(loadedExtensions), new Set([FETCH_EXTENSION, BITWISE_EXTENSION]));
+});
+
 test('canFetch', async () => {
     const vm = new VirtualMachine();
     setupUnsandboxedExtensionAPI(vm);