Finish shimming require & others in extension host

- try imports in 'evil' extension - don't need to put require and others back since the extension host is in its own process - replace `fetch` with `papi.fetch` - also remove `.promise` after #202
paranext · Jun 1, 2023 · 2594d96 · 2594d96
1 parent e3b3377
commit 2594d96
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 62 deletions.
diff --git a/extensions/lib/evil/evil.js b/extensions/lib/evil/evil.js
@@ -7,43 +7,80 @@ const papi = require('papi');
 
 const { logger } = papi;
 
-logger.info('Evil is importing! Mwahahaha');
-
-try {
-  // This will be blocked
-  const fs = require('fs');
-  logger.info(`Successfully imported fs! fs.readFileSync = ${fs.readFileSync}`);
-} catch (e) {
-  logger.info(e.message);
-}
+async function tryImports() {
+  logger.info('Evil is importing! Mwahahaha');
 
-try {
-  // This will be blocked and will suggest the papi.fetch api
-  const https = require('https');
-  logger.info(`Successfully imported https! ${https}`);
-} catch (e) {
-  logger.info(e.message);
-}
+  try {
+    // This will be blocked and will suggest the papi.storage api.
+    const fs = require('fs');
+    logger.error(`Successfully imported fs! fs.readFileSync = ${fs.readFileSync}`);
+  } catch (e) {
+    logger.info(e.message);
+  }
+
+  try {
+    // This will be blocked and will suggest the papi.fetch api.
+    const https = require('https');
+    logger.error(`Successfully imported https! ${https}`);
+  } catch (e) {
+    logger.info(e.message);
+  }
+
+  try {
+    // This should always work because `fetch` is replaced with `papi.fetch`.
+    fetch('https://bible-api.com/1%20thessalonians+5:16');
+    logger.info('Evil: fetch is working.');
+  } catch (e) {
+    logger.error(`Evil: Error on fetch! ${e}`);
+  }
+
+  try {
+    // This is just for testing and will throw an exception.
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    const xhr = new XMLHttpRequest();
+  } catch (e) {
+    logger.info(`Evil: Error on XMLHttpRequest! ${e}`);
+  }
 
-try {
-  // This is just for testing and will throw an exception
-  fetch('test');
-} catch (e) {
-  logger.info(`Evil: Error on fetch! ${e}`);
+  try {
+    // This is just for testing and will throw an exception.
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    const webSocket = new WebSocket();
+  } catch (e) {
+    logger.info(`Evil: Error on WebSocket! ${e}`);
+  }
+
+  try {
+    // This will be blocked and will suggest the papi.storage api.
+    const fs = await import('fs');
+    logger.error(`Successfully dynamically imported fs! fs.readFileSync = ${fs.readFileSync}`);
+  } catch (e) {
+    logger.info(`Evil: Error on dynamic import! ${e.message}`);
+  }
+
+  try {
+    // This should always work.
+    const verse = await papi.fetch('https://bible-api.com/1%20thessalonians+5:16');
+    const verseJson = await verse.json();
+    logger.info(`Evil: could papi.fetch verse 1TH 5:16 "${verseJson.text.replace(/\n/g, '')}"`);
+  } catch (e) {
+    logger.error(`Evil: Error on papi.fetch! ${e}`);
+  }
 }
 
-try {
-  // This is just for testing and will throw an exception
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  const xhr = new XMLHttpRequest();
-} catch (e) {
-  logger.info(`Evil: Error on XMLHttpRequest! ${e}`);
+tryImports();
+
+function activate() {
+  logger.info('Evil is activating...');
+  tryImports();
+  // 3 secs is timed to be after the extension service has finished initializing.
+  setTimeout(tryImports, 3000);
+  logger.info('Evil is finished activating!');
 }
 
-try {
-  // This is just for testing and will throw an exception
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  const webSocket = new WebSocket();
-} catch (e) {
-  logger.info(`Evil: Error on WebSocket! ${e}`);
+function deactivate() {
+  logger.info('Evil is deactivated.');
 }
+
+exports.activate = activate;
+exports.deactivate = deactivate;
diff --git a/src/extension-host/services/extension.service.ts b/src/extension-host/services/extension.service.ts
@@ -129,7 +129,8 @@ const getExtensions = async (): Promise<ExtensionInfo[]> => {
 /**
  * Loads an extension and runs its activate function.
  *
- * WARNING: This does not shim functionality out of extensions! Do not run this alone. Only run wrapped in activateExtensions()
+ * WARNING: This does not shim functionality out of extensions! Do not run this alone. Only run
+ *   wrapped in activateExtensions().
  * @param extension extension info for the extension to activate
  * @param extensionFilePath path to extension main file to import
  * @returns unsubscriber that deactivates the extension
@@ -138,7 +139,8 @@ const activateExtension = async (
   extension: ExtensionInfo,
   extensionFilePath: string,
 ): Promise<ActiveExtension> => {
-  // Import the extension file. Tell webpack to ignore it because extension files are not in the bundle and should not be looked up in the bundle
+  // Import the extension file. Tell webpack to ignore it because extension files are not in the
+  // bundle and should not be looked up in the bundle
   // DO NOT REMOVE THE webpackIgnore COMMENT. It is a webpack "Magic Comment" https://webpack.js.org/api/module-methods/#magic-comments
   const extensionModule = (await import(/* webpackIgnore: true */ extensionFilePath)) as IExtension;
 
@@ -190,47 +192,43 @@ const activateExtensions = async (extensions: ExtensionInfo[]): Promise<ActiveEx
 
   // Shim out require so extensions can't use it
   const requireOriginal = Module.prototype.require;
-  Module.prototype.require = ((fileName: string) => {
+  Module.prototype.require = ((moduleName: string) => {
     // Allow the extension to import papi
-    if (fileName === 'papi') return papi;
+    if (moduleName === 'papi') return papi;
 
     // Figure out if we are doing the import for the extension file in activateExtension
     const extensionFile = extensionsWithFiles.find(
       (extensionFileToCheck) =>
-        !extensionFileToCheck.hasBeenImported && extensionFileToCheck.filePath === fileName,
+        !extensionFileToCheck.hasBeenImported && extensionFileToCheck.filePath === moduleName,
     );
 
     if (extensionFile) {
-      // The file that is being imported is the extension file, so this hopefully means
-      // we are importing the extension file in activateExtension. Allow this and mark the extension as imported.
+      // The file that is being imported is the extension file, so this hopefully means we are
+      // importing the extension file in activateExtension. Allow this and mark the extension as
+      // imported.
       // TODO: an extension can probably import another extension's file and mess this up. Maybe try to find a better way
       extensionFile.hasBeenImported = true;
-      return requireOriginal(fileName);
+      return requireOriginal(moduleName);
     }
 
     // Disallow any imports within the extension
     // Tell the extension dev if there is an api similar to what they want to import
     const message = `Requiring other than papi is not allowed in extensions! ${getModuleSimilarApiMessage(
-      fileName,
+      moduleName,
     )}`;
     throw new Error(message);
   }) as typeof Module.prototype.require;
 
-  // Shim out internet access options in environments where they are defined so extensions can't use them
-  const fetchOriginal: typeof fetch | undefined = globalThis.fetch;
+  // Replace fetch with papi.fetch.
   // eslint-disable-next-line no-global-assign
-  globalThis.fetch = function fetchForbidden() {
-    throw new Error('Cannot use fetch! Try using papi.fetch');
-  };
+  globalThis.fetch = papi.fetch;
 
-  const xmlHttpRequestOriginal: typeof XMLHttpRequest | undefined = globalThis.XMLHttpRequest;
   // @ts-expect-error we want to remove XMLHttpRequest
   // eslint-disable-next-line no-global-assign
   globalThis.XMLHttpRequest = function XMLHttpRequestForbidden() {
     throw new Error('Cannot use XMLHttpRequest! Try using papi.fetch');
   };
 
-  const webSocketOriginal: typeof WebSocket | undefined = globalThis.WebSocket;
   // @ts-expect-error we want to remove WebSocket
   // eslint-disable-next-line no-global-assign
   globalThis.WebSocket = function WebSocketForbidden() {
@@ -243,28 +241,22 @@ const activateExtensions = async (extensions: ExtensionInfo[]): Promise<ActiveEx
       extensionsWithFiles.map((extensionWithFile) =>
         activateExtension(extensionWithFile.extension, extensionWithFile.filePath).catch((e) => {
           logger.error(
-            `Extension ${extensionWithFile.extension.name} threw while activating! ${e}`,
+            `Extension '${extensionWithFile.extension.name}' threw while activating! ${e}`,
           );
           return null;
         }),
       ),
     )
   ).filter((activeExtension) => activeExtension !== null) as ActiveExtension[];
 
-  // Put shimmed out modules and globals back so we can use them again
-  // TODO: replacing the original modules and globals almost confidently lets extensions wait and use them later. Serious security concern. Pls fix
-  Module.prototype.require = requireOriginal;
-  // eslint-disable-next-line no-global-assign
-  globalThis.fetch = fetchOriginal;
-  // eslint-disable-next-line no-global-assign
-  globalThis.XMLHttpRequest = xmlHttpRequestOriginal;
-  // eslint-disable-next-line no-global-assign
-  globalThis.WebSocket = webSocketOriginal;
-
   return extensionsActive;
 };
 
-/** Sets up the ExtensionService. Runs only once */
+/**
+ * Sets up the ExtensionService. Runs only once
+ *
+ * WARNING: import everything needed before this initialize as `require` becomes limited after.
+ */
 export const initialize = () => {
   if (initializePromise) return initializePromise;
 

diff --git a/src/shared/services/extension-asset.service.ts b/src/shared/services/extension-asset.service.ts
@@ -41,7 +41,7 @@ const initialize = async () => {
       async (extensionName: string, assetName: string) => {
         return getExtensionAsset(extensionName, assetName);
       },
-    ).promise;
+    );
 
     isInitialized = true;
   })();