Skip to content

Commit 2e0838c

Browse files
authored
Merge branch 'master' into APS-19734-npmrc-hardening
2 parents c6dc2c8 + 5a5b816 commit 2e0838c

6 files changed

Lines changed: 281 additions & 6 deletions

File tree

‎bin/commands/runs.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ module.exports = function run(args, rawArgs) {
212212
logger.debug("Completed setting the configs");
213213

214214
if(!isBrowserstackInfra) {
215-
if(process.env.BS_TESTOPS_BUILD_COMPLETED) {
215+
if(process.env.BS_TESTOPS_BUILD_COMPLETED === "true") {
216216
setEventListeners(bsConfig);
217217
}
218218

@@ -226,7 +226,7 @@ module.exports = function run(args, rawArgs) {
226226
if(process.env.BROWSERSTACK_TEST_ACCESSIBILITY === 'true') {
227227
setAccessibilityEventListeners(bsConfig);
228228
}
229-
if(process.env.BS_TESTOPS_BUILD_COMPLETED) {
229+
if(process.env.BS_TESTOPS_BUILD_COMPLETED === "true") {
230230
setEventListeners(bsConfig);
231231
}
232232
markBlockEnd('validateConfig');

‎bin/helpers/capabilityHelper.js‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ const { readCypressConfigFile } = require('./readCypressConfigUtil');
44

55
const logger = require("./logger").winstonLogger,
66
Constants = require("./constants"),
7-
Utils = require("./utils");
7+
Utils = require("./utils"),
8+
testhubUtils = require("../testhub/utils");
89

910
const caps = (bsConfig, zip) => {
1011
return new Promise(function (resolve, reject) {
@@ -131,6 +132,18 @@ const caps = (bsConfig, zip) => {
131132
obj.run_settings = JSON.stringify(bsConfig.run_settings);
132133
}
133134

135+
// The only route by which a cypress session can name its TestHub build: every session
136+
// this build spawns inherits these caps. Written unconditionally so an empty uuid records
137+
// that build start ran and had nothing to name, which an absent key cannot express.
138+
// "null" is the sentinel a failed build start leaves behind, not a uuid.
139+
const testhubBuildUuid = process.env.BROWSERSTACK_TESTHUB_UUID;
140+
obj.testhubBuildUuid = Utils.isUndefined(testhubBuildUuid) || testhubBuildUuid === "null"
141+
? ""
142+
: testhubBuildUuid;
143+
obj.buildProductMap = testhubUtils.getProductMap(bsConfig);
144+
145+
logger.debug(`TestHub build uuid stamped on caps: ${obj.testhubBuildUuid || "<empty>"}`);
146+
134147
obj.cypress_cli_user_agent = Utils.getUserAgent();
135148

136149
logger.info(`Cypress CLI User Agent: ${obj.cypress_cli_user_agent}`);

‎bin/testhub/testhubHandler.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ class TestHubHandler {
4141
const response = await nodeRequest( "POST", TESTHUB_CONSTANTS.TESTHUB_BUILD_API, data, config);
4242
const launchData = this.extractDataFromResponse(user_config, data, response, config);
4343
} catch (error) {
44-
console.log(error);
44+
logger.debug(`EXCEPTION IN BUILD START EVENT : ${error}`);
45+
testhubUtils.handleErrorForObservability(error.success === false ? error : null);
4546
if (error.success === false) { // non 200 response
46-
testhubUtils.logBuildError(error);
4747
return;
4848
}
4949

‎bin/testhub/utils.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ exports.setTestObservabilityVariables = (
8888
};
8989

9090
exports.handleErrorForObservability = (error = null) => {
91+
// Downstream reads isTestObservabilitySession(), not these ids, to decide whether
92+
// observability is live. extractDataFromResponse clears it inline for a 2xx carrying
93+
// success=false; this covers the paths that never get a usable response at all.
94+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = "false";
9195
process.env.BROWSERSTACK_TESTHUB_UUID = "null";
9296
process.env.BROWSERSTACK_TESTHUB_JWT = "null";
9397
process.env.BS_TESTOPS_BUILD_COMPLETED = "false";
@@ -164,7 +168,7 @@ exports.handleErrorForAccessibility = (user_config, error = null) => {
164168
};
165169

166170
exports.logBuildError = (error, product = "") => {
167-
if (error === undefined) {
171+
if (isUndefined(error)) {
168172
logger.error(`${product.toUpperCase()} Build creation failed`);
169173

170174
return;

‎test/unit/bin/helpers/capabilityHelper.js‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ const chai = require("chai"),
55

66
const capabilityHelper = require("../../../../bin/helpers/capabilityHelper"),
77
Constants = require("../../../../bin/helpers/constants"),
8+
testhubUtils = require("../../../../bin/testhub/utils"),
9+
o11yHelper = require("../../../../bin/testObservability/helper/helper"),
810
logger = require("../../../../bin/helpers/logger").winstonLogger;
911

1012
chai.use(chaiAsPromised);
@@ -562,6 +564,151 @@ describe("capabilityHelper.js", () => {
562564
});
563565
});
564566
});
567+
568+
context("testhub build attribution", () => {
569+
const bsConfig = {
570+
auth: {
571+
username: "random",
572+
access_key: "random",
573+
},
574+
browsers: [
575+
{
576+
browser: "chrome",
577+
os: "Windows 10",
578+
versions: ["78"],
579+
},
580+
],
581+
run_settings: {},
582+
};
583+
const productMap = {
584+
observability: true,
585+
accessibility: false,
586+
percy: false,
587+
automate: true,
588+
app_automate: false,
589+
};
590+
let productMapStub;
591+
let originalTesthubUuid;
592+
593+
beforeEach(() => {
594+
originalTesthubUuid = process.env.BROWSERSTACK_TESTHUB_UUID;
595+
productMapStub = sinon.stub(testhubUtils, "getProductMap").returns(productMap);
596+
});
597+
598+
afterEach(() => {
599+
productMapStub.restore();
600+
if (originalTesthubUuid === undefined) {
601+
delete process.env.BROWSERSTACK_TESTHUB_UUID;
602+
} else {
603+
process.env.BROWSERSTACK_TESTHUB_UUID = originalTesthubUuid;
604+
}
605+
});
606+
607+
it("stamps the testhub build uuid and the product map on the caps", () => {
608+
process.env.BROWSERSTACK_TESTHUB_UUID = "some-testhub-build-uuid";
609+
return capabilityHelper
610+
.caps(bsConfig, { zip_url: "bs://<random>" })
611+
.then(function (data) {
612+
let parsed_data = JSON.parse(data);
613+
chai.assert.equal(parsed_data.testhubBuildUuid, "some-testhub-build-uuid");
614+
chai.assert.deepEqual(parsed_data.buildProductMap, productMap);
615+
sinon.assert.calledWith(productMapStub, bsConfig);
616+
});
617+
});
618+
619+
it("stamps an empty testhub build uuid when build start produced none", () => {
620+
delete process.env.BROWSERSTACK_TESTHUB_UUID;
621+
return capabilityHelper
622+
.caps(bsConfig, { zip_url: "bs://<random>" })
623+
.then(function (data) {
624+
let parsed_data = JSON.parse(data);
625+
chai.assert.equal(parsed_data.testhubBuildUuid, "");
626+
chai.assert.isTrue(Object.prototype.hasOwnProperty.call(parsed_data, "testhubBuildUuid"));
627+
chai.assert.deepEqual(parsed_data.buildProductMap, productMap);
628+
});
629+
});
630+
});
631+
632+
// These exercise the REAL getProductMap rather than a stub, because the thing under test is
633+
// what the map SAYS, not that it is attached.
634+
context("testhub build attribution — product map reflects reality", () => {
635+
const ENV = ["BROWSERSTACK_TEST_OBSERVABILITY", "BROWSERSTACK_TESTHUB_UUID",
636+
"BROWSERSTACK_TEST_ACCESSIBILITY", "BROWSERSTACK_AUTOMATION"];
637+
let saved;
638+
639+
const bsConfigFor = (testObservability) => ({
640+
auth: { username: "random", access_key: "random" },
641+
browsers: [{ browser: "chrome", os: "Windows 10", versions: ["78"] }],
642+
run_settings: { cypress_config_file: "./cypress.config.js" },
643+
testObservability,
644+
});
645+
646+
beforeEach(() => {
647+
saved = {};
648+
ENV.forEach((k) => { saved[k] = process.env[k]; });
649+
delete process.env.BROWSERSTACK_TEST_OBSERVABILITY;
650+
process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false";
651+
process.env.BROWSERSTACK_AUTOMATION = "true";
652+
});
653+
654+
afterEach(() => {
655+
ENV.forEach((k) => {
656+
if (saved[k] === undefined) delete process.env[k];
657+
else process.env[k] = saved[k];
658+
});
659+
});
660+
661+
it("carries observability:false when the user explicitly disabled it in config", () => {
662+
const bsConfig = bsConfigFor(false);
663+
o11yHelper.setTestObservabilityFlags(bsConfig);
664+
chai.assert.equal(process.env.BROWSERSTACK_TEST_OBSERVABILITY, "false", "precondition");
665+
666+
return capabilityHelper
667+
.caps(bsConfig, { zip_url: "bs://<random>" })
668+
.then(function (data) {
669+
const parsed_data = JSON.parse(data);
670+
chai.assert.isFalse(parsed_data.buildProductMap.observability);
671+
chai.assert.equal(parsed_data.testhubBuildUuid, "");
672+
});
673+
});
674+
675+
it("carries observability:true when the user asked for it and build start succeeded", () => {
676+
const bsConfig = bsConfigFor(true);
677+
o11yHelper.setTestObservabilityFlags(bsConfig);
678+
process.env.BROWSERSTACK_TESTHUB_UUID = "a-real-build-uuid";
679+
680+
return capabilityHelper
681+
.caps(bsConfig, { zip_url: "bs://<random>" })
682+
.then(function (data) {
683+
const parsed_data = JSON.parse(data);
684+
chai.assert.isTrue(parsed_data.buildProductMap.observability);
685+
chai.assert.equal(parsed_data.testhubBuildUuid, "a-real-build-uuid");
686+
});
687+
});
688+
689+
it("flips observability to false and drops the null sentinel when build start failed", () => {
690+
const bsConfig = bsConfigFor(true);
691+
o11yHelper.setTestObservabilityFlags(bsConfig);
692+
chai.assert.equal(process.env.BROWSERSTACK_TEST_OBSERVABILITY, "true", "precondition");
693+
694+
const errorStub = sinon.stub(logger, "error");
695+
try {
696+
testhubUtils.handleErrorForObservability();
697+
} finally {
698+
errorStub.restore();
699+
}
700+
chai.assert.equal(process.env.BROWSERSTACK_TESTHUB_UUID, "null", "sentinel is what we guard against");
701+
702+
return capabilityHelper
703+
.caps(bsConfig, { zip_url: "bs://<random>" })
704+
.then(function (data) {
705+
const parsed_data = JSON.parse(data);
706+
chai.assert.isFalse(parsed_data.buildProductMap.observability);
707+
chai.assert.equal(parsed_data.testhubBuildUuid, "",
708+
'the "null" sentinel must never be stamped as a uuid');
709+
});
710+
});
711+
});
565712
});
566713

567714
describe("addCypressZipStartLocation", () => {

‎test/unit/bin/testhub/utils.js‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
const chai = require("chai"),
2+
sinon = require("sinon");
3+
4+
const testhubUtils = require("../../../../bin/testhub/utils"),
5+
logger = require("../../../../bin/helpers/logger").winstonLogger;
6+
7+
describe("testhub/utils.js", () => {
8+
const OBSERVABILITY_ENV = [
9+
"BROWSERSTACK_TEST_OBSERVABILITY",
10+
"BROWSERSTACK_TESTHUB_UUID",
11+
"BROWSERSTACK_TESTHUB_JWT",
12+
"BS_TESTOPS_BUILD_COMPLETED",
13+
"BS_TESTOPS_JWT",
14+
"BS_TESTOPS_BUILD_HASHED_ID",
15+
"BS_TESTOPS_ALLOW_SCREENSHOTS",
16+
"BROWSERSTACK_TEST_ACCESSIBILITY",
17+
"BROWSERSTACK_AUTOMATION",
18+
];
19+
let saved;
20+
21+
beforeEach(() => {
22+
saved = {};
23+
OBSERVABILITY_ENV.forEach((k) => { saved[k] = process.env[k]; });
24+
});
25+
26+
afterEach(() => {
27+
OBSERVABILITY_ENV.forEach((k) => {
28+
if (saved[k] === undefined) delete process.env[k];
29+
else process.env[k] = saved[k];
30+
});
31+
});
32+
33+
describe("getProductMap", () => {
34+
it("reports observability false when the flag is not set to true", () => {
35+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = "false";
36+
process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false";
37+
process.env.BROWSERSTACK_AUTOMATION = "true";
38+
39+
chai.assert.deepEqual(testhubUtils.getProductMap({}), {
40+
observability: false,
41+
accessibility: false,
42+
percy: false,
43+
automate: true,
44+
app_automate: false,
45+
});
46+
});
47+
48+
it("reports observability true only while the flag says so", () => {
49+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = "true";
50+
process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false";
51+
process.env.BROWSERSTACK_AUTOMATION = "true";
52+
53+
chai.assert.isTrue(testhubUtils.getProductMap({}).observability);
54+
});
55+
});
56+
57+
describe("handleErrorForObservability", () => {
58+
let errorStub;
59+
60+
beforeEach(() => { errorStub = sinon.stub(logger, "error"); });
61+
afterEach(() => { errorStub.restore(); });
62+
63+
it("turns observability off in the product map when build start fails", () => {
64+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = "true";
65+
process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false";
66+
process.env.BROWSERSTACK_AUTOMATION = "true";
67+
chai.assert.isTrue(testhubUtils.getProductMap({}).observability, "precondition");
68+
69+
testhubUtils.handleErrorForObservability();
70+
71+
chai.assert.equal(process.env.BROWSERSTACK_TEST_OBSERVABILITY, "false");
72+
chai.assert.isFalse(testhubUtils.getProductMap({}).observability);
73+
});
74+
75+
it("leaves the uuid as the null sentinel and marks the build not completed", () => {
76+
process.env.BROWSERSTACK_TESTHUB_UUID = "some-uuid";
77+
process.env.BS_TESTOPS_BUILD_COMPLETED = "true";
78+
79+
testhubUtils.handleErrorForObservability();
80+
81+
chai.assert.equal(process.env.BROWSERSTACK_TESTHUB_UUID, "null");
82+
chai.assert.equal(process.env.BS_TESTOPS_BUILD_COMPLETED, "false");
83+
});
84+
85+
it("does not report observability as still enabled to shouldProcessEventForTesthub", () => {
86+
process.env.BROWSERSTACK_TEST_OBSERVABILITY = "true";
87+
process.env.BROWSERSTACK_TEST_ACCESSIBILITY = "false";
88+
89+
testhubUtils.handleErrorForObservability();
90+
91+
chai.assert.isFalse(testhubUtils.shouldProcessEventForTesthub());
92+
});
93+
});
94+
95+
describe("logBuildError", () => {
96+
let errorStub;
97+
98+
beforeEach(() => { errorStub = sinon.stub(logger, "error"); });
99+
afterEach(() => { errorStub.restore(); });
100+
101+
it("logs a readable message when there is no error object at all", () => {
102+
testhubUtils.logBuildError(undefined, "observability");
103+
sinon.assert.calledWith(errorStub, "OBSERVABILITY Build creation failed");
104+
});
105+
106+
it("treats a null error the same as a missing one", () => {
107+
testhubUtils.logBuildError(null, "observability");
108+
sinon.assert.calledWith(errorStub, "OBSERVABILITY Build creation failed");
109+
});
110+
});
111+
});

0 commit comments

Comments
 (0)