From 55e2461df1f5025b82cb8905713e901248547064 Mon Sep 17 00:00:00 2001 From: Tyler Brandt Date: Thu, 17 May 2018 14:13:33 -0700 Subject: [PATCH 1/6] Fix vulnerabilities by removing request dependency Summary: [`srcclr`](https://optimizely.sourceclear.io/teams/3OOu2k/scans/3772139?login=saml) flags `request` and some of its transitive dependencies as vulnerable. Since we are only barely using it, we can switch to the native node modules `http`/`https` instead. The result is a [[ https://optimizely.sourceclear.io/teams/3OOu2k/scans/3772270?login=saml | clean run ]]! ``` $ srcclr scan . SourceClear scanning engine ready Running the NPM scanner Scanning completed Found 12997 lines of code Processing results... Processing results complete Summary Report Scan ID 614c7376-d1ae-4d77-ad5f-3078f6a17917 Scan Date & Time May 16 2018 04:26PM PDT Account type PRO Scan engine 2.14.7 (latest 2.14.7) Analysis time 12 seconds User tbrandt Project /Users/tbrandt/optly/Projects/javascript-sdk/packages/optimizely-sdk Package Manager(s) NPM Open-Source Libraries Total Libraries 5 Direct Libraries 5 Transitive Libraries 0 Vulnerable Libraries 0 Third Party Code 77.4% Security With Vulnerable Methods 0 High Risk Vulnerabilities 0 Medium Risk Vulnerabilities 0 Low Risk Vulnerabilities 0 Licenses Unique Library Licenses 3 Libraries Using GPL 0 Libraries With No License 1 Libraries With Multiple Licenses 1 Full Report Details https://optimizely.sourceclear.io/teams/3OOu2k/scans/3772270?login=saml ``` Also add srcclr.yml with `scope: production` so we can scan more easily in the correct way. Test Plan: Existing automated tests Reviewers: matt.carroll, ola.nordstrom, michael.hood, ali, greeshma Reviewed By: ali, greeshma JIRA Issues: OASIS-2776 Differential Revision: https://phabricator.optimizely.com/D19829 --- .../optimizely-sdk/lib/optimizely/index.js | 10 ++--- .../plugins/event_dispatcher/index.node.js | 43 +++++++++++++------ .../event_dispatcher/index.node.tests.js | 33 +++++++++++--- packages/optimizely-sdk/package.json | 1 - packages/optimizely-sdk/srcclr.yml | 1 + 5 files changed, 63 insertions(+), 25 deletions(-) create mode 100644 packages/optimizely-sdk/srcclr.yml diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 15b03393c..7277a3d38 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -362,7 +362,7 @@ Optimizely.prototype.getForcedVariation = function(experimentKey, userId) { Optimizely.prototype.__validateInputs = function(stringInputs, userAttributes, eventTags) { try { var inputKeys = Object.keys(stringInputs); - for (var index=0; index < inputKeys.length; index++) { + for (var index = 0; index < inputKeys.length; index++) { var key = inputKeys[index]; if (!stringValidator.validate(stringInputs[key])) { throw new Error(sprintf(ERROR_MESSAGES.INVALID_INPUT_FORMAT, MODULE_NAME, key)); @@ -449,12 +449,12 @@ Optimizely.prototype.__notActivatingExperiment = function(experimentKey, userId) Optimizely.prototype.__dispatchEvent = function (eventToDispatch, callback) { var eventDispatcherResponse = this.eventDispatcher.dispatchEvent(eventToDispatch, callback); //checking that response value is a promise, not a request object - if (typeof eventDispatcherResponse == "object" && !eventDispatcherResponse.hasOwnProperty('uri')) { + if (!fns.isEmpty(eventDispatcherResponse) && typeof eventDispatcherResponse.then === 'function') { eventDispatcherResponse.then(function() { callback(); }); } -} +}; /** * Filters out attributes/eventTags with null or undefined values @@ -464,11 +464,11 @@ Optimizely.prototype.__dispatchEvent = function (eventToDispatch, callback) { Optimizely.prototype.__filterEmptyValues = function (map) { for (var key in map) { if (map.hasOwnProperty(key) && (map[key] === null || map[key] === undefined)) { - delete map[key] + delete map[key]; } } return map; -} +}; /** * Returns true if the feature is enabled for the given user. diff --git a/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js b/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js index 49bcdbb30..b3583d7dc 100644 --- a/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js +++ b/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js @@ -1,5 +1,5 @@ /** - * Copyright 2016-2017, Optimizely + * Copyright 2016-2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,9 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -var request = require('request'); - -var POST_HEADERS = {'content-type': 'application/json'}; +var http = require('http'); +var https = require('https'); +var url = require('url'); module.exports = { /** @@ -25,23 +25,42 @@ module.exports = { * @param {Object} eventObj.params parameters to pass to the request * @param {string} eventObj.httpVerb the HTTP request method type * @param {function} callback callback to execute - * @return {Promise} the payload from the request + * @return {ClientRequest} ClientRequest object which made the request */ dispatchEvent: function(eventObj, callback) { + // Non-POST requests not supported + if (eventObj.httpVerb !== 'POST') { + callback(new Error('httpVerb not supported: ' + eventObj.httpVerb)); + return; + } + + var parsedUrl = url.parse(eventObj.url); + var path = parsedUrl.path; + if (parsedUrl.query) { + path += '?' + parsedUrl.query; + } + + var dataString = JSON.stringify(eventObj.params); + var requestOptions = { - uri: eventObj.url, - body: eventObj.params, - headers: POST_HEADERS, - method: eventObj.httpVerb, - json: true, + host: parsedUrl.host, + path: parsedUrl.path, + method: 'POST', + headers: { + 'content-type': 'application/json', + 'content-length': dataString.length.toString(), + } }; var requestCallback = function(error, response, body) { if (response && response.statusCode && response.statusCode >= 200 && response.statusCode < 400 && callback && typeof callback == 'function') { callback(); } - } + }; - return request(requestOptions, requestCallback); + var req = (parsedUrl.protocol === 'http:' ? http : https).request(requestOptions, requestCallback); + req.write(dataString); + req.end(); + return req; } }; diff --git a/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js b/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js index 84f0c90dc..f38d5966f 100644 --- a/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js +++ b/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js @@ -24,7 +24,7 @@ describe('lib/plugins/event_dispatcher/node', function() { describe('dispatchEvent', function() { var stubCallback = { callback: function() {} - } + }; beforeEach(function() { sinon.stub(stubCallback, 'callback'); @@ -43,7 +43,7 @@ describe('lib/plugins/event_dispatcher/node', function() { it('should send a POST request with the specified params', function(done) { var eventObj = { url: 'https://cdn.com/event', - body: { + params: { id: 123, }, httpVerb: 'POST', @@ -55,14 +55,14 @@ describe('lib/plugins/event_dispatcher/node', function() { done(); }) .on('error', function(error) { - assert.fail('status code okay', 'status code not okay', "") - }) + assert.fail('status code okay', 'status code not okay', ''); + }); }); it('should execute the callback passed to event dispatcher', function(done) { var eventObj = { url: 'https://cdn.com/event', - body: { + params: { id: 123, }, httpVerb: 'POST', @@ -74,10 +74,29 @@ describe('lib/plugins/event_dispatcher/node', function() { sinon.assert.calledOnce(stubCallback.callback); }) .on('error', function(error) { - assert.fail('status code okay', 'status code not okay', "") - }) + assert.fail('status code okay', 'status code not okay', ''); + }); }); + it('rejects GET httpVerb', function(done) { + var eventObj = { + url: 'https://cdn.com/event', + params: { + id: 123, + }, + httpVerb: 'GET', + }; + + var callback = function(err) { + if (err) { + done(); + } else { + done(new Error('expected error not thrown')); + } + }; + + eventDispatcher.dispatchEvent(eventObj, callback); + }); }); }); }); diff --git a/packages/optimizely-sdk/package.json b/packages/optimizely-sdk/package.json index b702bc0ed..c44ea3b91 100644 --- a/packages/optimizely-sdk/package.json +++ b/packages/optimizely-sdk/package.json @@ -35,7 +35,6 @@ "json-schema": "^0.2.3", "lodash": "^4.0.0", "murmurhash": "0.0.2", - "request": "~2.83.0", "sprintf": "^0.1.5", "uuid": "^3.0.1" }, diff --git a/packages/optimizely-sdk/srcclr.yml b/packages/optimizely-sdk/srcclr.yml new file mode 100644 index 000000000..5941418eb --- /dev/null +++ b/packages/optimizely-sdk/srcclr.yml @@ -0,0 +1 @@ +scope: production From 2466852c6ad0ff9d9c4dc7dd3a55f87e28f3acd6 Mon Sep 17 00:00:00 2001 From: Tyler Brandt Date: Fri, 18 May 2018 11:49:58 -0700 Subject: [PATCH 2/6] add node v9 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 2f61b33f9..8ff8fcf83 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ node_js: - '4' - '6' - '8' -- node +- '9' branches: only: - master From 288f3cf7266aa1ed29e25975f83a8b0bc4a10e62 Mon Sep 17 00:00:00 2001 From: Tyler Brandt Date: Fri, 18 May 2018 17:40:17 -0700 Subject: [PATCH 3/6] add back node target --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 8ff8fcf83..af564ebc9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ node_js: - '6' - '8' - '9' +- node branches: only: - master From b937393c758ce25d21f42e41917f3c1520a17878 Mon Sep 17 00:00:00 2001 From: Tyler Brandt Date: Fri, 18 May 2018 17:45:42 -0700 Subject: [PATCH 4/6] explicit node 10 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index af564ebc9..37fc419fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ node_js: - '6' - '8' - '9' -- node +- '10' branches: only: - master From 4cf9d7c8156a405279d23dad7de95db66da46505 Mon Sep 17 00:00:00 2001 From: Tyler Brandt Date: Mon, 21 May 2018 10:00:37 -0700 Subject: [PATCH 5/6] update docs --- .../lib/plugins/event_dispatcher/index.node.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js b/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js index b3583d7dc..4d8b2881f 100644 --- a/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js +++ b/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js @@ -22,10 +22,10 @@ module.exports = { * Dispatch an HTTP request to the given url and the specified options * @param {Object} eventObj Event object containing * @param {string} eventObj.url the url to make the request to - * @param {Object} eventObj.params parameters to pass to the request - * @param {string} eventObj.httpVerb the HTTP request method type + * @param {Object} eventObj.params parameters to pass to the request (i.e. in the POST body) + * @param {string} eventObj.httpVerb the HTTP request method type. only POST is supported. * @param {function} callback callback to execute - * @return {ClientRequest} ClientRequest object which made the request + * @return {ClientRequest|undefined} ClientRequest object which made the request, or undefined if no request was made (error) */ dispatchEvent: function(eventObj, callback) { // Non-POST requests not supported From d10b62b02aa7039cc2d790f0600e156ef12d68f5 Mon Sep 17 00:00:00 2001 From: Tyler Brandt Date: Mon, 21 May 2018 10:01:31 -0700 Subject: [PATCH 6/6] update copyright --- .../lib/plugins/event_dispatcher/index.node.tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js b/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js index f38d5966f..1c4907b9c 100644 --- a/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js +++ b/packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js @@ -1,5 +1,5 @@ /** - * Copyright 2016-2017, Optimizely + * Copyright 2016-2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.