From b8ccb47dc340c12e8e8ba26fd37086f4128dfd4e Mon Sep 17 00:00:00 2001 From: Ashfaqbs <105435085+Ashfaqbs@users.noreply.github.com> Date: Tue, 22 Sep 2026 22:21:05 +0530 Subject: [PATCH] fix: honor proxyUser/proxyPass when downloading the Local binary proxyHost/proxyPort were threaded through to every binary-download request (the async in-process path, and the two spawned child scripts), but proxyUser/proxyPass were only ever forwarded to the already-running BrowserStackLocal binary's own --proxy-user/--proxy-pass flags - Local.js's conf object never carried them, so an authenticating proxy accepted the binary's own traffic but rejected the download of the binary itself. Thread proxyUser/proxyPass through the same four call sites that already handle proxyHost/proxyPort, building an HttpsProxyAgent `auth` option ("user:pass", matching what https-proxy-agent expects for the Proxy-Authorization header) from them. For the two spawned children (download.js, fetchDownloadSourceUrl.js), pass the credentials via env instead of argv, mirroring this codebase's existing BROWSERSTACK_LOCAL_AUTH_TOKEN pattern - argv is readable via `ps`/ /proc//cmdline, env is not. Added 5 regression tests. Confirmed each fails against the pre-fix code and passes with the fix. Fixes #164 --- lib/Local.js | 4 + lib/LocalBinary.js | 21 +++-- lib/download.js | 12 ++- lib/fetchDownloadSourceUrl.js | 18 ++-- lib/fetchDownloadSourceUrlAsync.js | 11 +-- test/local.js | 127 +++++++++++++++++++++++++++++ 6 files changed, 171 insertions(+), 22 deletions(-) diff --git a/lib/Local.js b/lib/Local.js index 05d29e0..077afa3 100644 --- a/lib/Local.js +++ b/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; diff --git a/lib/LocalBinary.js b/lib/LocalBinary.js index 00e1748..fa1e42c 100644 --- a/lib/LocalBinary.js +++ b/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 { diff --git a/lib/download.js b/lib/download.js index dde74a2..e94172c 100644 --- a/lib/download.js +++ b/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//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 diff --git a/lib/fetchDownloadSourceUrl.js b/lib/fetchDownloadSourceUrl.js index 6b5d37c..31fd926 100644 --- a/lib/fetchDownloadSourceUrl.js +++ b/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//cmdline, whereas /proc//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//cmdline, whereas + /proc//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 { diff --git a/lib/fetchDownloadSourceUrlAsync.js b/lib/fetchDownloadSourceUrlAsync.js index ad6e97a..e33d439 100644 --- a/lib/fetchDownloadSourceUrlAsync.js +++ b/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 { diff --git a/test/local.js b/test/local.js index abe162c..83f3bd8 100644 --- a/test/local.js +++ b/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