Merge pull request #720 from snyk/fix/broker-var-sub-in-universal-broker

fix: body and header var sub in universal mode

lib/common/relay/prepareRequest.ts

@@ -11,6 +11,7 @@ import {
   signGitHubCommit,
   validateGitHubTreePayload,
 } from '../../client/scm';
+import { getConfigForIdentifier } from '../config/universal';
 
 export interface PostFilterPreparingRequestError {
   status: number;
@@ -87,7 +88,12 @@ export const prepareRequestFromFilterResult = async (
       logContext.bodyVarsSubstitution = parsedBody.BROKER_VAR_SUB;
       for (const path of parsedBody.BROKER_VAR_SUB) {
         let source = undefsafe(parsedBody, path); // get the value
-        source = replace(source, options.config); // replace the variables
+        source = replace(
+          source,
+          options.config.universalBrokerEnabled
+            ? getConfigForIdentifier(brokerToken, options.config)
+            : options.config,
+        ); // replace the variables
         undefsafe(parsedBody, path, source); // put it back in
       }
       payload.body = JSON.stringify(parsedBody);
@@ -103,7 +109,12 @@ export const prepareRequestFromFilterResult = async (
     logContext.headerVarsSubstitution = payload.headers['x-broker-var-sub'];
     for (const path of payload.headers['x-broker-var-sub'].split(',')) {
       let source = undefsafe(payload.headers, path.trim()); // get the value
-      source = replace(source, options.config); // replace the variables
+      source = replace(
+        source,
+        options.config.universalBrokerEnabled
+          ? getConfigForIdentifier(brokerToken, options.config)
+          : options.config,
+      ); // replace the variables
       undefsafe(payload.headers, path.trim(), source); // put it back in
     }
   }
@@ -173,17 +184,32 @@ export const prepareRequestFromFilterResult = async (
   }
 
   if (
-    gitHubCommitSigningEnabled(options.config, {
-      method: payload.method,
-      url: payload.url,
-    })
+    gitHubCommitSigningEnabled(
+      options.config.universalBrokerEnabled
+        ? getConfigForIdentifier(brokerToken, options.config)
+        : options.config,
+      {
+        method: payload.method,
+        url: payload.url,
+      },
+    )
   ) {
     try {
-      payload.body = await signGitHubCommit(options.config, payload.body);
+      payload.body = await signGitHubCommit(
+        options.config.universalBrokerEnabled
+          ? getConfigForIdentifier(brokerToken, options.config)
+          : options.config,
+        payload.body,
+      );
     } catch (error) {
       logger.error({ error }, 'error while signing github commit');
     }
   }
+  if (options.config && options.config.LOG_ENABLE_BODY === 'true') {
+    logContext.requestBody = payload.body;
+  }
+  logContext.requestHeaders = payload.headers;
+  logger.debug(logContext, 'Prepared request');
 
   const req = {
     url: result.url,

test/unit/relay-response-body-universal.test.ts

@@ -0,0 +1,181 @@
+const PORT = 8001;
+process.env.BROKER_SERVER_URL = `http://localhost:${PORT}`;
+
+jest.mock('../../lib/common/http/request');
+import { WebSocketConnection } from '../../lib/client/types/client';
+import { makeRequestToDownstream } from '../../lib/common/http/request';
+
+// eslint-disable-next-line @typescript-eslint/ban-ts-comment
+// @ts-ignore
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+const mockedFn = makeRequestToDownstream.mockImplementation((data) => {
+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+  // @ts-ignore
+  return data;
+});
+
+import { forwardWebSocketRequest as relay } from '../../lib/common/relay/forwardWebsocketRequest';
+import {
+  LoadedClientOpts,
+  LoadedServerOpts,
+} from '../../lib/common/types/options';
+
+const dummyWebsocketHandler: WebSocketConnection = {
+  destroy: () => {
+    return;
+  },
+  latency: 0,
+  options: {
+    ping: 0,
+    pong: 0,
+    queueSize: Infinity,
+    reconnect: '',
+    stategy: '',
+    timeout: 100,
+    transport: '',
+  },
+  send: () => {},
+  serverId: '0',
+  socket: {},
+  supportedIntegrationType: 'github',
+  transport: '',
+  url: '',
+  on: () => {},
+  readyState: 3,
+};
+
+const dummyLoadedFilters = {
+  private: () => {
+    return { url: '/', auth: '', stream: true };
+  },
+  public: () => {
+    return { url: '/', auth: '', stream: true };
+  },
+};
+
+describe('body relay', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  afterAll(() => {
+    delete process.env.BROKER_SERVER_URL;
+    jest.clearAllMocks();
+  });
+
+  it('relay swaps body values found in BROKER_VAR_SUB', (done) => {
+    expect.hasAssertions();
+
+    const brokerToken = 'test-broker';
+
+    const config = {
+      universalBrokerEnabled: true,
+      connections: {
+        myconn: {
+          identifier: brokerToken,
+          HOST: 'localhost',
+          PORT: '8001',
+        },
+      },
+    };
+    const options: LoadedClientOpts | LoadedServerOpts = {
+      filters: {
+        private: [
+          {
+            method: 'any',
+            url: '/*',
+          },
+        ],
+        public: [],
+      },
+      config,
+      port: 8001,
+      loadedFilters: dummyLoadedFilters,
+    };
+
+    const route = relay(options, dummyWebsocketHandler)(brokerToken);
+
+    const body = {
+      BROKER_VAR_SUB: ['url'],
+      url: '${HOST}:${PORT}/webhook',
+    };
+
+    route(
+      {
+        url: '/',
+        method: 'POST',
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
+        body: Buffer.from(JSON.stringify(body)),
+        headers: {},
+      },
+      () => {
+        expect(makeRequestToDownstream).toHaveBeenCalledTimes(1);
+        const arg = mockedFn.mock.calls[0][0];
+        expect(JSON.parse(arg.body).url).toEqual(
+          `${config.connections.myconn.HOST}:${config.connections.myconn.PORT}/webhook`,
+        );
+
+        done();
+      },
+    );
+  });
+
+  it('relay does NOT swap body values found in BROKER_VAR_SUB if disabled', (done) => {
+    expect.hasAssertions();
+
+    const brokerToken = 'test-broker';
+
+    const config = {
+      universalBrokerEnabled: true,
+      connections: {
+        myconn: {
+          identifier: brokerToken,
+          HOST: 'localhost',
+          PORT: '8001',
+        },
+      },
+      disableBodyVarsSubstitution: true,
+      brokerServerUrl: 'http://localhost:8001',
+    };
+
+    const options: LoadedClientOpts | LoadedServerOpts = {
+      filters: {
+        private: [
+          {
+            method: 'any',
+            url: '/*',
+          },
+        ],
+        public: [],
+      },
+      config,
+      port: 8001,
+      loadedFilters: dummyLoadedFilters,
+    };
+    const route = relay(options, dummyWebsocketHandler)(brokerToken);
+
+    const body = {
+      BROKER_VAR_SUB: ['url'],
+      url: '${HOST}:${PORT}/webhook',
+    };
+
+    route(
+      {
+        url: '/',
+        method: 'POST',
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
+        body: Buffer.from(JSON.stringify(body)),
+        headers: {},
+      },
+      () => {
+        expect(makeRequestToDownstream).toHaveBeenCalledTimes(1);
+        const arg = mockedFn.mock.calls[0][0];
+        expect(JSON.parse(arg.body).url).toEqual('${HOST}:${PORT}/webhook');
+
+        done();
+      },
+    );
+  });
+});