Merge pull request #732 from snyk/IA-98-app-risk-integrations-broker

fix: url encoded header fix
snyk · Mar 11, 2024 · 371ca84 · 371ca84
2 parents ff171b0 + 05dd2fb
commit 371ca84
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 14 deletions.
diff --git a/defaultFilters/apprisk.json b/defaultFilters/apprisk.json
@@ -11,25 +11,25 @@
       {
         "//": "Get All Project Details",
         "method": "GET",
-        "path": "/projects",
+        "path": "/cxrestapi/projects",
         "origin": "https://${CHECKMARX}"
       },
       {
         "//": "Get Remote Source Settings for GIT",
         "method": "GET",
-        "path": "/projects/:id/sourceCode/remoteSettings/git",
+        "path": "/cxrestapi/projects/:id/sourceCode/remoteSettings/git",
         "origin": "https://${CHECKMARX}"
       },
       {
         "//": "Get All Scans for Project",
         "method": "GET",
-        "path": "/sast/scans",
+        "path": "/cxrestapi/sast/scans",
         "origin": "https://${CHECKMARX}"
       },
       {
         "//": "Get Statistic Results by Scan Id",
         "method": "GET",
-        "path": "/sast/scans/:id/resultsStatistics",
+        "path": "/cxrestapi/sast/scans/:id/resultsStatistics",
         "origin": "https://${CHECKMARX}"
       }
     ]

diff --git a/lib/common/relay/prepareRequest.ts b/lib/common/relay/prepareRequest.ts
@@ -96,6 +96,8 @@ export const prepareRequestFromFilterResult = async (
         ); // replace the variables
         undefsafe(parsedBody, path, source); // put it back in
       }
+      //Remove the BROKER_VAR_SUB for the request body
+      delete parsedBody.BROKER_VAR_SUB;
       payload.body = JSON.stringify(parsedBody);
     }
   }
@@ -205,19 +207,31 @@ export const prepareRequestFromFilterResult = async (
       logger.error({ error }, 'error while signing github commit');
     }
   }
+  const urlencoded = 'application/x-www-form-urlencoded';
   if (
     payload.headers &&
-    payload.headers['x-broker-content-type'] ===
-      'application/x-www-form-urlencoded'
+    payload.headers['x-broker-content-type'] === urlencoded
   ) {
-    payload.headers['Content-Type'] = 'application/x-www-form-urlencoded';
+    const contentTypeHeader = 'content-type';
+    //avoid duplication for content-type headers
+    Object.keys(payload.headers).forEach((header) => {
+      if (header.toLowerCase() === contentTypeHeader) {
+        delete payload.headers[header];
+      }
+    });
+    payload.headers[contentTypeHeader] = urlencoded;
     if (payload.body) {
       const jsonBody = JSON.parse(payload.body) as Record<string, any>;
       const params = new URLSearchParams();
       for (const [key, value] of Object.entries(jsonBody)) {
         params.append(key, value.toString());
       }
       payload.body = params.toString();
+
+      //updating the content length after converting the body
+      const encoder = new TextEncoder();
+      const byteArray = encoder.encode(payload.body);
+      payload.headers['Content-length'] = byteArray.length;
     }
   }
 

diff --git a/test/functional/server-client.test.ts b/test/functional/server-client.test.ts
@@ -150,7 +150,6 @@ describe('proxy requests originating from behind the broker server', () => {
 
     expect(response.status).toEqual(200);
     expect(response.data).toStrictEqual({
-      BROKER_VAR_SUB: ['swap.me'],
       swap: { me: 'client:broker-token-12345' },
     });
   });

diff --git a/test/unit/relay-response-body-form-url-encoded.test.ts b/test/unit/relay-response-body-form-url-encoded.test.ts
@@ -110,7 +110,7 @@ describe('body relay', () => {
         expect(makeRequestToDownstream).toHaveBeenCalledTimes(1);
         const arg = mockedFn.mock.calls[0][0];
         expect(arg.body).toEqual(
-          `BROKER_VAR_SUB=url&url=${config.HOST}%3A${config.PORT}%2Fwebhook`,
+          `url=${config.HOST}%3A${config.PORT}%2Fwebhook`,
         );
 
         done();

diff --git a/test/unit/relay-response-body-universal-form-url-encoded.test.ts b/test/unit/relay-response-body-universal-form-url-encoded.test.ts
@@ -116,11 +116,11 @@ describe('body relay', () => {
         expect(makeRequestToDownstream).toHaveBeenCalledTimes(1);
         const arg = mockedFn.mock.calls[0][0];
 
-        expect(arg.headers['Content-Type']).toEqual(
+        expect(arg.headers['content-type']).toEqual(
           'application/x-www-form-urlencoded',
         );
         expect(arg.body).toEqual(
-          `BROKER_VAR_SUB=url&url=${config.connections.myconn.HOST}%3A${config.connections.myconn.PORT}%2Fwebhook`,
+          `url=${config.connections.myconn.HOST}%3A${config.connections.myconn.PORT}%2Fwebhook`,
         );
 
         done();
@@ -185,4 +185,64 @@ describe('body relay', () => {
       },
     );
   });
+
+  it('calculate content type after converting request body', (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',
+    };
+    const headers = {
+      'x-broker-content-type': 'application/x-www-form-urlencoded',
+    };
+
+    route(
+      {
+        url: '/',
+        method: 'POST',
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
+        body: Buffer.from(JSON.stringify(body)),
+        headers: headers,
+      },
+      () => {
+        expect(makeRequestToDownstream).toHaveBeenCalledTimes(1);
+        const arg = mockedFn.mock.calls[0][0];
+
+        expect(arg.headers['Content-length']).toEqual(arg.body.length);
+
+        done();
+      },
+    );
+  });
 });
diff --git a/test/unit/relay-response-headers-form-url-headers.test.ts b/test/unit/relay-response-headers-form-url-headers.test.ts
@@ -102,7 +102,7 @@ describe('header relay', () => {
       () => {
         expect(makeRequestToDownstream).toHaveBeenCalledTimes(1);
         const arg = mockedFn.mock.calls[0][0];
-        expect(arg.headers['Content-Type']).toEqual(
+        expect(arg.headers['content-type']).toEqual(
           'application/x-www-form-urlencoded',
         );
         expect(arg.headers['private-token']).toEqual(

diff --git a/test/unit/relay-response-headers-universal-form-url-headers.test.ts b/test/unit/relay-response-headers-universal-form-url-headers.test.ts
@@ -93,6 +93,7 @@ describe('header relay', () => {
 
     const headers = {
       'x-broker-content-type': 'application/x-www-form-urlencoded',
+      'Content-Type': 'application/json',
       'x-broker-var-sub': 'private-token,replaceme',
       donttouch: 'not to be changed ${VALUE}',
       'private-token': 'Bearer ${SECRET_TOKEN}',
@@ -108,7 +109,8 @@ describe('header relay', () => {
       () => {
         expect(makeRequestToDownstream).toHaveBeenCalledTimes(1);
         const arg = mockedFn.mock.calls[0][0];
-        expect(arg.headers['Content-Type']).toEqual(
+        expect(arg.headers['Content-Type']).toBeUndefined();
+        expect(arg.headers['content-type']).toEqual(
           'application/x-www-form-urlencoded',
         );
         expect(arg.headers['private-token']).toEqual(
@@ -174,7 +176,7 @@ describe('header relay', () => {
       () => {
         expect(makeRequestToDownstream).toHaveBeenCalledTimes(1);
         const arg = mockedFn.mock.calls[0][0];
-        expect(arg.headers['Content-Type']).toEqual(
+        expect(arg.headers['content-type']).toEqual(
           'application/x-www-form-urlencoded',
         );
         expect(arg.headers['private-token']).toEqual('Bearer ${SECRET_TOKEN}');