fix: honor proxyUser/proxyPass when downloading the Local binary

lib/Local.js

@@ -295,6 +295,10 @@ function Local(){
       if(this.proxyHost && this.proxyPort){
         conf.proxyHost = this.proxyHost;
         conf.proxyPort = this.proxyPort;
+        if (this.proxyUser && this.proxyPass) {
+          conf.proxyUser = this.proxyUser;
+          conf.proxyPass = this.proxyPass;
+        }
       }
       if (this.useCaCertificate) {
         conf.useCaCertificate = this.useCaCertificate;

lib/LocalBinary.js

@@ -70,6 +70,12 @@ function LocalBinary(){
     if (this.key) {
       env.BROWSERSTACK_LOCAL_AUTH_TOKEN = this.key;
     }
+    /* Same reasoning as the auth token above: proxy credentials go through the
+       environment, not argv. */
+    if (conf.proxyUser && conf.proxyPass) {
+      env.BROWSERSTACK_LOCAL_PROXY_USER = conf.proxyUser;
+      env.BROWSERSTACK_LOCAL_PROXY_PASS = conf.proxyPass;
+    }
     const obj = childProcess.spawnSync(cmd, opts, { env: env });
     if(obj.stdout.length > 0) {
       this.sourceURL = obj.stdout.toString().replace(/\n+$/, '');
@@ -100,7 +106,7 @@ function LocalBinary(){
       downloadErrorMessage = this.downloadErrorMessage || this.downloadState.errorMessage;
     }
 
-    fetchDownloadSourceUrlAsync(this.key, this.bsHost, downloadFallback, downloadErrorMessage, conf.proxyHost, conf.proxyPort, conf.useCaCertificate, (err, sourceURL) => {
+    fetchDownloadSourceUrlAsync(this.key, this.bsHost, downloadFallback, downloadErrorMessage, conf.proxyHost, conf.proxyPort, conf.useCaCertificate, conf.proxyUser, conf.proxyPass, (err, sourceURL) => {
       if (err) return callback(err);
       this.sourceURL = sourceURL;
       this.downloadState.sourceURL = sourceURL;
@@ -197,6 +203,10 @@ function LocalBinary(){
     try{
       const userAgent = [packageName, version].join('/');
       const env = Object.assign({ 'USER_AGENT': userAgent }, process.env);
+      if (conf.proxyUser && conf.proxyPass) {
+        env.BROWSERSTACK_LOCAL_PROXY_USER = conf.proxyUser;
+        env.BROWSERSTACK_LOCAL_PROXY_PASS = conf.proxyPass;
+      }
       const obj = childProcess.spawnSync(cmd, opts, { env: env });
       let output;
       if(obj.stdout.length > 0) {
@@ -236,10 +246,11 @@ function LocalBinary(){
 
       var options = url.parse(this.httpPath);
       if(conf.proxyHost && conf.proxyPort) {
-        options.agent = new HttpsProxyAgent({
-          host: conf.proxyHost,
-          port: conf.proxyPort
-        });
+        var proxyOpts = { host: conf.proxyHost, port: conf.proxyPort };
+        if (conf.proxyUser && conf.proxyPass) {
+          proxyOpts.auth = `${conf.proxyUser}:${conf.proxyPass}`;
+        }
+        options.agent = new HttpsProxyAgent(proxyOpts);
       }
       if (conf.useCaCertificate) {
         try {

lib/download.js

@@ -6,6 +6,9 @@ const https = require('https'),
   { isUndefined } = require('./util');
 
 const binaryPath = process.argv[2], httpPath = process.argv[3], proxyHost = process.argv[4], proxyPort = process.argv[5], useCaCertificate = process.argv[6];
+/* Proxy credentials are read from the environment, never argv: argv is
+   world-readable via `ps` / /proc/<pid>/cmdline. Mirrors fetchDownloadSourceUrl.js. */
+const proxyUser = process.env.BROWSERSTACK_LOCAL_PROXY_USER, proxyPass = process.env.BROWSERSTACK_LOCAL_PROXY_PASS;
 
 var fileStream = fs.createWriteStream(binaryPath);
 
@@ -15,10 +18,11 @@ var options = url.parse(httpPath);
    arrive here as the *string* "undefined" — which is truthy, and previously
    built a proxy agent pointing at the host "undefined". */
 if(!isUndefined(proxyHost) && !isUndefined(proxyPort)) {
-  options.agent = new HttpsProxyAgent({
-    host: proxyHost,
-    port: proxyPort
-  });
+  const proxyOpts = { host: proxyHost, port: proxyPort };
+  if (!isUndefined(proxyUser) && !isUndefined(proxyPass)) {
+    proxyOpts.auth = `${proxyUser}:${proxyPass}`;
+  }
+  options.agent = new HttpsProxyAgent(proxyOpts);
 }
 
 /* Applied regardless of whether a proxy is configured: this is the caller's TLS

lib/fetchDownloadSourceUrl.js

@@ -3,10 +3,11 @@ const https = require('https'),
   HttpsProxyAgent = require('https-proxy-agent'),
   { isUndefined } = require('./util');
 
-/* The auth token is read from the environment, never from argv: argv is world-readable
-   via `ps` / /proc/<pid>/cmdline, whereas /proc/<pid>/environ is restricted to the
-   owning user. Keep it out of this argument list. */
-const authToken = process.env.BROWSERSTACK_LOCAL_AUTH_TOKEN, bsHost = process.argv[2], proxyHost = process.argv[5], proxyPort = process.argv[6], useCaCertificate = process.argv[7], downloadFallback = process.argv[3], downloadErrorMessage = process.argv[4];
+/* The auth token and proxy credentials are read from the environment, never from
+   argv: argv is world-readable via `ps` / /proc/<pid>/cmdline, whereas
+   /proc/<pid>/environ is restricted to the owning user. Keep them out of this
+   argument list. */
+const authToken = process.env.BROWSERSTACK_LOCAL_AUTH_TOKEN, proxyUser = process.env.BROWSERSTACK_LOCAL_PROXY_USER, proxyPass = process.env.BROWSERSTACK_LOCAL_PROXY_PASS, bsHost = process.argv[2], proxyHost = process.argv[5], proxyPort = process.argv[6], useCaCertificate = process.argv[7], downloadFallback = process.argv[3], downloadErrorMessage = process.argv[4];
 
 let body = '', data = {'auth_token': authToken};
 const options = {
@@ -25,10 +26,11 @@ if (downloadFallback == 'true') {
 }
 
 if(!isUndefined(proxyHost) && !isUndefined(proxyPort)) {
-  options.agent = new HttpsProxyAgent({
-    host: proxyHost,
-    port: proxyPort
-  });
+  const proxyOpts = { host: proxyHost, port: proxyPort };
+  if (!isUndefined(proxyUser) && !isUndefined(proxyPass)) {
+    proxyOpts.auth = `${proxyUser}:${proxyPass}`;
+  }
+  options.agent = new HttpsProxyAgent(proxyOpts);
 }
 if (!isUndefined(useCaCertificate)) {
   try {

lib/fetchDownloadSourceUrlAsync.js

@@ -6,7 +6,7 @@ const https = require('https'),
 
 const packageName = 'browserstack-local-nodejs';
 
-function fetchDownloadSourceUrlAsync(authToken, bsHost, downloadFallback, downloadErrorMessage, proxyHost, proxyPort, useCaCertificate, callback) {
+function fetchDownloadSourceUrlAsync(authToken, bsHost, downloadFallback, downloadErrorMessage, proxyHost, proxyPort, useCaCertificate, proxyUser, proxyPass, callback) {
   let body = '', data = {'auth_token': authToken};
   const userAgent = [packageName, version].join('/');
   const options = {
@@ -25,10 +25,11 @@ function fetchDownloadSourceUrlAsync(authToken, bsHost, downloadFallback, downlo
   }
 
   if(!isUndefined(proxyHost) && !isUndefined(proxyPort)) {
-    options.agent = new HttpsProxyAgent({
-      host: proxyHost,
-      port: proxyPort
-    });
+    const proxyOpts = { host: proxyHost, port: proxyPort };
+    if (!isUndefined(proxyUser) && !isUndefined(proxyPass)) {
+      proxyOpts.auth = `${proxyUser}:${proxyPass}`;
+    }
+    options.agent = new HttpsProxyAgent(proxyOpts);
   }
   if (!isUndefined(useCaCertificate)) {
     try {

test/local.js

@@ -491,6 +491,133 @@ describe('LocalBinary', function () {
   });
 });
 
+// Regression tests for https://github.com/browserstack/browserstack-local-nodejs/issues/164:
+// proxyUser/proxyPass were accepted by Local's config and forwarded to the
+// BrowserStackLocal *binary* itself (--proxy-user/--proxy-pass), but never reached
+// the node-side binary download, so an authenticating proxy rejected the download
+// even though the same config worked for everything the binary does afterwards.
+describe('Proxy authentication for binary download', function () {
+  var https = require('https');
+  var childProcess = require('child_process');
+  var Local = require('../lib/Local');
+  var binary, sandBox, tempDownloadPath;
+
+  beforeEach(function () {
+    binary = new LocalBinary();
+    sandBox = sinon.sandbox.create();
+    tempDownloadPath = path.join(process.cwd(), 'download-proxy-auth');
+  });
+
+  afterEach(function () {
+    sandBox.restore();
+    rimraf.sync(tempDownloadPath);
+  });
+
+  it('sends proxy credentials on the CONNECT agent used by the async download', function (done) {
+    sandBox.stub(binary, 'getDownloadPath', function (conf, retries, callback) {
+      callback(null, 'https://example.invalid/fake-binary');
+    });
+    sandBox.stub(https, 'get', function (options) {
+      check(done, function () {
+        expect(options.agent.proxy.host).to.equal('127.0.0.1');
+        expect(String(options.agent.proxy.port)).to.equal('8080');
+        expect(options.agent.proxy.auth).to.equal('proxyuser:proxypass');
+      });
+      return { on: function () { return this; } };
+    });
+
+    binary.download({
+      proxyHost: '127.0.0.1',
+      proxyPort: 8080,
+      proxyUser: 'proxyuser',
+      proxyPass: 'proxypass'
+    }, tempDownloadPath, function () {});
+  });
+
+  it('does not set agent auth when no proxy credentials are configured', function (done) {
+    sandBox.stub(binary, 'getDownloadPath', function (conf, retries, callback) {
+      callback(null, 'https://example.invalid/fake-binary');
+    });
+    sandBox.stub(https, 'get', function (options) {
+      check(done, function () {
+        expect(options.agent.proxy.auth).to.equal(undefined);
+      });
+      return { on: function () { return this; } };
+    });
+
+    binary.download({ proxyHost: '127.0.0.1', proxyPort: 8080 }, tempDownloadPath, function () {});
+  });
+
+  it('passes proxy credentials to the spawned download.js child via env, not argv', function () {
+    var spawnStub = sandBox.stub(childProcess, 'spawnSync', function () {
+      return { stdout: Buffer.from('ok'), stderr: Buffer.from('') };
+    });
+    sandBox.stub(fs, 'existsSync', function () { return true; });
+    sandBox.stub(fs, 'chmodSync', function () {});
+
+    binary.downloadSync({
+      proxyHost: '127.0.0.1',
+      proxyPort: 8080,
+      proxyUser: 'proxyuser',
+      proxyPass: 'proxypass'
+    }, tempDownloadPath, 0);
+
+    var call = spawnStub.getCall(0);
+    expect(call.args[1]).to.not.contain('proxyuser');
+    expect(call.args[1]).to.not.contain('proxypass');
+    expect(call.args[2].env.BROWSERSTACK_LOCAL_PROXY_USER).to.equal('proxyuser');
+    expect(call.args[2].env.BROWSERSTACK_LOCAL_PROXY_PASS).to.equal('proxypass');
+  });
+
+  it('passes proxy credentials to the spawned fetchDownloadSourceUrl.js child via env, not argv', function () {
+    var spawnStub = sandBox.stub(childProcess, 'spawnSync', function () {
+      return { stdout: Buffer.from('https://example.invalid'), stderr: Buffer.from('') };
+    });
+
+    binary.getSourceUrlSync({
+      proxyHost: '127.0.0.1',
+      proxyPort: 8080,
+      proxyUser: 'proxyuser',
+      proxyPass: 'proxypass'
+    }, 0);
+
+    var call = spawnStub.getCall(0);
+    expect(call.args[1]).to.not.contain('proxyuser');
+    expect(call.args[1]).to.not.contain('proxypass');
+    expect(call.args[2].env.BROWSERSTACK_LOCAL_PROXY_USER).to.equal('proxyuser');
+    expect(call.args[2].env.BROWSERSTACK_LOCAL_PROXY_PASS).to.equal('proxypass');
+  });
+
+  it('forwards proxyUser/proxyPass from Local config through to the download child process env', function () {
+    var spawnStub = sandBox.stub(childProcess, 'spawnSync', function () {
+      return { stdout: Buffer.from('https://example.invalid'), stderr: Buffer.from('') };
+    });
+    // Make the sync path succeed on the first attempt instead of retrying:
+    // downloadSync's spawnSync is stubbed and never actually writes a binary,
+    // so without this the real retry loop (async via fs.unlink) keeps firing
+    // child processes after the test has already finished and its stubs are
+    // restored.
+    sandBox.stub(fs, 'existsSync', function () { return true; });
+    sandBox.stub(fs, 'chmodSync', function () {});
+
+    var bsLocal = new Local();
+    bsLocal.proxyHost = '127.0.0.1';
+    bsLocal.proxyPort = 8080;
+    bsLocal.proxyUser = 'proxyuser';
+    bsLocal.proxyPass = 'proxypass';
+
+    // No callback -> the sync path, which ends in the same spawnSync used by
+    // getSourceUrlSync/downloadSync, whichever this hits first with an empty
+    // binary directory.
+    bsLocal.getBinaryPath();
+
+    expect(spawnStub.called).to.equal(true);
+    var call = spawnStub.getCall(0);
+    expect(call.args[2].env.BROWSERSTACK_LOCAL_PROXY_USER).to.equal('proxyuser');
+    expect(call.args[2].env.BROWSERSTACK_LOCAL_PROXY_PASS).to.equal('proxypass');
+  });
+});
+
 // Regression tests: the binary-download fallback signalling used to live on
 // process.env, so (a) a value planted in process.env steered the download to an
 // arbitrary host with no validation, and (b) a failure on one Local instance bled