Merge pull request #793 from snyk/fix/ghsa-plugin-and-plugin-loading-…

…in-img fix: [unibroker] github srv app plgn and plugins loading
snyk · Jul 15, 2024 · 6d9f659 · 6d9f659
2 parents ec4afc4 + 0989b75
commit 6d9f659
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 14 deletions.
diff --git a/defaultFilters/github-server-app.json b/defaultFilters/github-server-app.json
@@ -355,7 +355,7 @@
         "//": "rate limit check",
         "method": "GET",
         "path": "/rate_limit",
-        "origin": "https://${GITHUB_API}"
+        "origin": "https://x-access-token:${ACCESS_TOKEN}@${GITHUB_API}"
       },
       {
         "//": "allow meta lookup on the api version",

diff --git a/lib/client/brokerClientPlugins/pluginManager.ts b/lib/client/brokerClientPlugins/pluginManager.ts
@@ -10,6 +10,7 @@ export const loadPlugins = async (pluginsFolderPath: string, clientOpts) => {
     clientOpts.config.plugins.set(type, []);
   });
   try {
+    logger.debug({}, `Loading plugins from ${pluginsFolderPath}`);
     if (existsSync(pluginsFolderPath)) {
       const pluginsFiles = await readdir(pluginsFolderPath);
       for (const pluginFile of pluginsFiles.filter((filename) =>

diff --git a/lib/client/brokerClientPlugins/plugins/githubServerAppAuth.ts b/lib/client/brokerClientPlugins/plugins/githubServerAppAuth.ts
@@ -38,7 +38,8 @@ export class Plugin extends BrokerPlugin {
         connectionConfig &&
         (!connectionConfig.GITHUB_APP_CLIENT_ID ||
           !connectionConfig.GITHUB_APP_PRIVATE_PEM_PATH ||
-          !connectionConfig.GITHUB_APP_INSTALLATION_ID)
+          !connectionConfig.GITHUB_APP_INSTALLATION_ID ||
+          !connectionConfig.GITHUB_APP_ID)
       ) {
         throw new Error(
           `Missing environment variable(s) for plugin (GITHUB_APP_CLIENT_ID, GITHUB_APP_PRIVATE_PEM_PATH, GITHUB_APP_INSTALLATION_ID)`,
@@ -59,7 +60,7 @@ export class Plugin extends BrokerPlugin {
       connectionConfig.JWT_TOKEN = this._getJWT(
         Math.floor(now / 1000), // Current time in seconds
         connectionConfig.GITHUB_APP_PRIVATE_PEM_PATH,
-        connectionConfig.GITHUB_APP_CLIENT_ID,
+        connectionConfig.GITHUB_APP_ID,
       );
       this._setJWTLifecycleHandler(now, connectionConfig);
 
@@ -71,8 +72,9 @@ export class Plugin extends BrokerPlugin {
       connectionConfig.ACCESS_TOKEN = JSON.parse(
         connectionConfig.accessToken,
       ).token;
-
-      this._setAccessTokenLifecycleHandler(connectionConfig);
+      if (connectionConfig.ACCESS_TOKEN) {
+        this._setAccessTokenLifecycleHandler(connectionConfig);
+      }
     } catch (err) {
       this.logger.error(
         { err },
@@ -87,7 +89,7 @@ export class Plugin extends BrokerPlugin {
   _getJWT(
     nowInSeconds: number,
     privatePemPath: string,
-    githubAppClientId: string,
+    githubAppId: string,
   ): string {
     // Read the contents of the PEM file
     const privatePem = readFileSync(privatePemPath, 'utf8');
@@ -96,9 +98,8 @@ export class Plugin extends BrokerPlugin {
     const payload = {
       iat: nowInSeconds - 60, // Issued at time (60 seconds in the past)
       exp: nowInSeconds + this.JWT_TTL / 1000, // Expiration time (10 minutes from now)
-      iss: githubAppClientId, // GitHub App's client ID
+      iss: githubAppId, // GitHub App's ID
     };
-
     // Generate the JWT
     return sign(payload, privateKey, { algorithm: 'RS256' });
   }
@@ -119,7 +120,7 @@ export class Plugin extends BrokerPlugin {
             connectionConfig.JWT_TOKEN = await this._getJWT(
               Math.floor(timeoutHandlerNow / 1000),
               connectionConfig.GITHUB_APP_PRIVATE_PEM_PATH,
-              connectionConfig.GITHUB_APP_CLIENT_ID,
+              connectionConfig.GITHUB_APP_ID,
             );
             if (process.env.NODE_ENV != 'test') {
               timeoutHandlerId = setTimeout(
@@ -172,6 +173,11 @@ export class Plugin extends BrokerPlugin {
       };
 
       const oauthResponse = await makeRequestToDownstream(request);
+      if (oauthResponse.statusCode && oauthResponse.statusCode > 299) {
+        throw new Error(
+          `Unexpected error code ${oauthResponse.statusCode}: ${oauthResponse.body}`,
+        );
+      }
       const accessToken = oauthResponse.body ?? '';
       return accessToken;
     } catch (err) {

diff --git a/lib/client/index.ts b/lib/client/index.ts
@@ -26,9 +26,9 @@ import { loadAllFilters } from '../common/filter/filtersAsync';
 import { ClientOpts, LoadedClientOpts } from '../common/types/options';
 import { websocketConnectionSelectorMiddleware } from './routesHandler/websocketConnectionMiddlewares';
 import { getClientConfigMetadata } from './config/configHelpers';
-import { findProjectRoot } from '../common/config/config';
 import { loadPlugins } from './brokerClientPlugins/pluginManager';
 import { manageWebsocketConnections } from './connectionsManager/manager';
+import { findPluginFolder } from '../common/config/config';
 
 process.on('uncaughtException', (error) => {
   if (error.message == 'read ECONNRESET') {
@@ -65,11 +65,16 @@ export const main = async (clientOpts: ClientOpts) => {
 
     await validateMinimalConfig(clientOpts);
     if (clientOpts.config.universalBrokerEnabled) {
-      const pluginsFolderPath = `${findProjectRoot(
-        __dirname,
-      )}/dist/lib/client/brokerClientPlugins/plugins`;
+      const pluginsFolderPath = await findPluginFolder(
+        __dirname ?? process.cwd(),
+        'brokerClientPlugins',
+      );
+      if (!pluginsFolderPath) {
+        throw new Error('Unable to load plugins - plugins folder not found.');
+      }
+
       clientOpts.config.plugins = await loadPlugins(
-        pluginsFolderPath,
+        `${pluginsFolderPath}/plugins`,
         clientOpts,
       );
     } else {

diff --git a/lib/common/config/config.ts b/lib/common/config/config.ts
@@ -34,6 +34,30 @@ export const findProjectRoot = (startDir: string): string | null => {
   return null;
 };
 
+export const findPluginFolder = async (
+  dirPath: string,
+  targetFolder: string,
+): Promise<string | null> => {
+  const files = await fs.promises.readdir(dirPath);
+
+  for (const file of files) {
+    const filePath = path.join(dirPath, file);
+    const stat = await fs.promises.stat(filePath);
+
+    if (stat.isDirectory()) {
+      if (file === targetFolder) {
+        return filePath;
+      }
+      const result = await findPluginFolder(filePath, targetFolder);
+      if (result) {
+        return result;
+      }
+    }
+  }
+
+  return null;
+};
+
 export const loadBrokerConfig = async (localConfigForTest?) => {
   dotenv.config({
     path: path.join(process.cwd(), '.env'),

diff --git a/test/unit/plugins/brokerPlugins/github-server-app.test.ts b/test/unit/plugins/brokerPlugins/github-server-app.test.ts
@@ -39,6 +39,7 @@ describe('Github Server App Plugin', () => {
 
   it('startUp plugin method errors if invalid pem path', async () => {
     const config = {
+      GITHUB_APP_ID: '123',
       GITHUB_APP_CLIENT_ID: '123',
       GITHUB_APP_INSTALLATION_ID: '123',
       GITHUB_APP_PRIVATE_PEM_PATH: '/invalid/path',