diff --git a/bin/helpers/utils.js b/bin/helpers/utils.js index a6903720..c15cc69d 100644 --- a/bin/helpers/utils.js +++ b/bin/helpers/utils.js @@ -31,7 +31,7 @@ const usageReporting = require("./usageReporting"), pkg = require('../../package.json'), transports = require('./logger').transports, o11yHelpers = require('../testObservability/helper/helper'), - { OBSERVABILITY_ENV_VARS, TEST_OBSERVABILITY_REPORTER } = require('../testObservability/helper/constants'); + { OBSERVABILITY_ENV_VARS, TEST_OBSERVABILITY_REPORTER, remoteReporterApiUrl } = require('../testObservability/helper/constants'); const { default: axios } = require("axios"); const { shouldProcessEventForTesthub } = require("../testhub/utils"); @@ -610,6 +610,10 @@ exports.setSystemEnvs = (bsConfig) => { logger.error(`Error in adding accessibility configs ${error}`) } + // Remote reporter must post to this build's collector, not its package default. Kept out of the + // try below, which swallows errors: a rejected override must fail rather than fall back silently. + envKeys['BROWSERSTACK_TEST_OBSERVABILITY_API_URL'] = remoteReporterApiUrl(); + try { OBSERVABILITY_ENV_VARS.forEach(key => { envKeys[key] = process.env[key]; diff --git a/bin/testObservability/helper/constants.js b/bin/testObservability/helper/constants.js index d3013f40..c2cb0c8f 100644 --- a/bin/testObservability/helper/constants.js +++ b/bin/testObservability/helper/constants.js @@ -3,7 +3,37 @@ const path = require('path'); exports.consoleHolder = Object.assign({},console); exports.BATCH_SIZE = 1000; exports.BATCH_INTERVAL = 2000; -exports.API_URL = 'https://collector-observability.browserstack.com'; +const DEFAULT_API_URL = 'https://collector-observability.browserstack.com'; + +// Requests to the collector carry BS_TESTOPS_JWT, so an http:// override would put the build's token +// on the wire in cleartext. Request urls are also built as `${base}/${path}`, so a query or fragment +// would swallow the endpoint. Reject such values instead of quietly using the default collector. +const httpsOverride = (name) => { + const value = process.env[name]; + if (!value) return undefined; + let parsed; + // The value can carry credentials, and callers log the error, so never echo it back. + try { + parsed = new URL(value); + } catch (err) { + throw new Error(`${name} must be a valid https:// URL`); + } + if (parsed.protocol !== 'https:' || !parsed.hostname) { + throw new Error(`${name} must be an https:// URL`); + } + if (parsed.search || parsed.hash) { + throw new Error(`${name} must not contain a query or fragment`); + } + // Normalise to origin + path with no trailing separator: a stray `/`, `?` or `#` would otherwise + // turn the appended endpoint into a query or fragment. + return `${parsed.origin}${parsed.pathname}`.replace(/\/+$/, ''); +}; + +// Overridable so the remote reporter (own install of this package) can target the launching cli's collector +exports.API_URL = httpsOverride('BROWSERSTACK_TEST_OBSERVABILITY_API_URL') || DEFAULT_API_URL; +// Collector handed to the reporter on the remote machine; separate override for when it cannot reach API_URL +exports.remoteReporterApiUrl = () => httpsOverride('BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL') + || httpsOverride('BROWSERSTACK_TEST_OBSERVABILITY_API_URL') || DEFAULT_API_URL; exports.IPC_EVENTS = { LOG: 'testObservability:cypressLog', diff --git a/test/unit/bin/helpers/utils.js b/test/unit/bin/helpers/utils.js index 480cb5f4..cac8faf5 100644 --- a/test/unit/bin/helpers/utils.js +++ b/test/unit/bin/helpers/utils.js @@ -961,6 +961,93 @@ describe('utils', () => { delete process.env.CYPRESS_TEST_1; delete process.env.cypress_test_2; }); + + const URL_ENV_VARS = [ + 'BROWSERSTACK_TEST_OBSERVABILITY_API_URL', + 'BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL', + ]; + let savedUrlEnvs = {}; + + beforeEach(() => { + URL_ENV_VARS.forEach((key) => { + savedUrlEnvs[key] = process.env[key]; + delete process.env[key]; + }); + }); + + afterEach(() => { + URL_ENV_VARS.forEach((key) => { + if (savedUrlEnvs[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedUrlEnvs[key]; + } + }); + }); + + it('forwards the observability collector url for the remote reporter', () => { + let bsConfig = { run_settings: { env: null } }; + + utils.setSystemEnvs(bsConfig); + expect(bsConfig.run_settings.system_env_vars) + .to.be.an('array') + .that.includes('BROWSERSTACK_TEST_OBSERVABILITY_API_URL=https://collector-observability.browserstack.com'); + }); + + it('forwards an overridden observability collector url', () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY_API_URL = 'https://collector-example.bsstag.com'; + let bsConfig = { run_settings: { env: null } }; + + utils.setSystemEnvs(bsConfig); + expect(bsConfig.run_settings.system_env_vars) + .to.be.an('array') + .that.includes('BROWSERSTACK_TEST_OBSERVABILITY_API_URL=https://collector-example.bsstag.com'); + }); + + it('prefers a separate remote collector url for the remote reporter', () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY_API_URL = 'https://collector-private.bsstag.com'; + process.env.BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL = 'https://collector-public.bsstag.com'; + let bsConfig = { run_settings: { env: null } }; + + utils.setSystemEnvs(bsConfig); + expect(bsConfig.run_settings.system_env_vars) + .to.be.an('array') + .that.includes('BROWSERSTACK_TEST_OBSERVABILITY_API_URL=https://collector-public.bsstag.com'); + }); + + it('rejects a collector url that is not https', () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL = 'http://collector-public.bsstag.com'; + let bsConfig = { run_settings: { env: null } }; + + expect(() => utils.setSystemEnvs(bsConfig)).to.throw(/must be an https:\/\/ URL/); + }); + + it('rejects a collector url carrying a query or fragment', () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL = 'https://collector-public.bsstag.com/#staging'; + let bsConfig = { run_settings: { env: null } }; + + expect(() => utils.setSystemEnvs(bsConfig)).to.throw(/must not contain a query or fragment/); + }); + + it('normalises a bare query or fragment delimiter', () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL = 'https://collector-public.bsstag.com?'; + let bsConfig = { run_settings: { env: null } }; + + utils.setSystemEnvs(bsConfig); + expect(bsConfig.run_settings.system_env_vars) + .to.be.an('array') + .that.includes('BROWSERSTACK_TEST_OBSERVABILITY_API_URL=https://collector-public.bsstag.com'); + }); + + it('trims a trailing slash so endpoint paths are not doubled', () => { + process.env.BROWSERSTACK_TEST_OBSERVABILITY_REMOTE_API_URL = 'https://collector-public.bsstag.com/'; + let bsConfig = { run_settings: { env: null } }; + + utils.setSystemEnvs(bsConfig); + expect(bsConfig.run_settings.system_env_vars) + .to.be.an('array') + .that.includes('BROWSERSTACK_TEST_OBSERVABILITY_API_URL=https://collector-public.bsstag.com'); + }); }); describe('fixCommaSeparatedString', () => {