Skip to content

Commit a620d87

Browse files
noomorphnoomorph
authoredMar 7, 2025··
fix: CVE-2024-21538 by migrating to promisify-child-process #4658 (#4729)
1 parent 4af265d commit a620d87

File tree

18 files changed

+273
-217
lines changed

18 files changed

+273
-217
lines changed
 

‎detox/local-cli/utils/frameworkUtils.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
const os = require('os');
22
const path = require('path');
33

4-
const { spawn } = require('child-process-promise');
54
const fs = require('fs-extra');
65

76
const detox = require('../../internals');
7+
const { spawnAndLog } = require('../../src/utils/childProcess');
88
const { getFrameworkDirPath, getXCUITestRunnerDirPath } = require('../../src/utils/environment');
99

10-
1110
const frameworkBuildScript = '../../scripts/build_local_framework.ios.sh';
1211
const xcuitestBuildScript = '../../scripts/build_local_xcuitest.ios.sh';
1312

@@ -26,7 +25,7 @@ async function execBuildScript(targetPath, scriptPath, descriptor) {
2625
const scriptFullPath = path.join(__dirname, scriptPath);
2726

2827
try {
29-
await spawn(scriptFullPath, [], { stdio: 'inherit' });
28+
await spawnAndLog(scriptFullPath, [], { stdio: 'inherit' });
3029
} catch (error) {
3130
detox.log.error(`Error while building ${descriptor}:\n${error}`);
3231
throw error;

‎detox/package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
"prettier": "^3.1.1",
6565
"react-native": "0.76.3",
6666
"react-native-codegen": "^0.0.8",
67-
"typescript": "^5.3.3",
67+
"typescript": "~5.3.3",
6868
"wtfnode": "^0.9.1"
6969
},
7070
"dependencies": {
@@ -73,14 +73,14 @@
7373
"bunyan-debug-stream": "^3.1.0",
7474
"caf": "^15.0.1",
7575
"chalk": "^4.0.0",
76-
"child-process-promise": "^2.2.0",
7776
"detox-copilot": "^0.0.27",
7877
"execa": "^5.1.1",
7978
"find-up": "^5.0.0",
8079
"fs-extra": "^11.0.0",
8180
"funpermaproxy": "^1.1.0",
8281
"glob": "^8.0.3",
8382
"ini": "^1.3.4",
83+
"promisify-child-process": "^4.1.2",
8484
"jest-environment-emit": "^1.0.8",
8585
"json-cycle": "^1.3.0",
8686
"lodash": "^4.17.11",

‎detox/src/android/AndroidExpect.test.js

+12
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,18 @@ describe('AndroidExpect', () => {
427427
});
428428

429429
describe('WebElement Actions', () => {
430+
let handle;
431+
432+
beforeEach(() => {
433+
jest.useFakeTimers({ doNotFake: ['clearInterval', 'setInterval'] });
434+
handle = setInterval(() => jest.runAllTimers(), 5);
435+
});
436+
437+
afterEach(() => {
438+
clearInterval(handle);
439+
jest.useRealTimers();
440+
});
441+
430442
it('tap', async () => {
431443
await e.web.element(e.by.web.id('id')).tap();
432444
await e.web.element(e.by.web.className('className')).tap();

‎detox/src/devices/allocation/drivers/android/emulator/launchEmulatorProcess.js

+28-14
Original file line numberDiff line numberDiff line change
@@ -31,23 +31,37 @@ function launchEmulatorProcess(emulatorExec, adb, emulatorLaunchCommand) {
3131

3232
log = log.child({ child_pid: childProcessPromise.childProcess.pid });
3333

34-
adb.waitForDevice(emulatorLaunchCommand.adbName).then(() => childProcessPromise._cpResolve());
34+
// Create a deferred promise that resolves when the device is ready
35+
let resolveEmulatorReady;
36+
const emulatorReadyPromise = new Promise(resolve => {
37+
resolveEmulatorReady = resolve;
38+
});
3539

36-
return childProcessPromise.then(() => true).catch((err) => {
37-
detach();
40+
// Wait for the device to be ready
41+
adb.waitForDevice(emulatorLaunchCommand.adbName).then(() => {
42+
resolveEmulatorReady();
43+
});
3844

39-
if (childProcessOutput.includes(`There's another emulator instance running with the current AVD`)) {
40-
return false;
41-
}
45+
// Use Promise.race to resolve with the first one to complete - either the emulator process exits
46+
// or the emulator device is ready
47+
return Promise.race([childProcessPromise, emulatorReadyPromise])
48+
.then(() => true)
49+
.catch((err) => {
50+
detach();
4251

43-
log.error({ event: 'SPAWN_FAIL', error: true, err }, err.message);
44-
log.error({ event: 'SPAWN_FAIL', stderr: true }, childProcessOutput);
45-
throw err;
46-
}).then((coldBoot) => {
47-
detach();
48-
log.debug({ event: 'SPAWN_SUCCESS', stdout: true }, childProcessOutput);
49-
return coldBoot;
50-
});
52+
if (childProcessOutput && childProcessOutput.includes(`There's another emulator instance running with the current AVD`)) {
53+
return false;
54+
}
55+
56+
log.error({ event: 'SPAWN_FAIL', error: true, err }, err.message);
57+
log.error({ event: 'SPAWN_FAIL', stderr: true }, childProcessOutput);
58+
throw err;
59+
})
60+
.then((coldBoot) => {
61+
detach();
62+
log.debug({ event: 'SPAWN_SUCCESS', stdout: true }, childProcessOutput);
63+
return coldBoot;
64+
});
5165
}
5266

5367
module.exports = { launchEmulatorProcess };

‎detox/src/devices/common/drivers/android/exec/ADB.js

+1
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ class ADB {
375375
...this.defaultExecOptions,
376376
...spawnOptions,
377377
capture: ['stdout'],
378+
encoding: 'utf8',
378379
};
379380
return spawnWithRetriesAndLogs(this.adbBin, _flags, _spawnOptions);
380381
}

‎detox/src/devices/common/drivers/android/exec/ADB.test.js

+2
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ describe('ADB', () => {
133133
retries: 3,
134134
timeout: 60000,
135135
capture: ['stdout'],
136+
encoding: 'utf8',
136137
};
137138
await adb.install(deviceId, 'path/to/bin.apk');
138139

@@ -248,6 +249,7 @@ describe('ADB', () => {
248249
retries: 3,
249250
timeout: 60000,
250251
capture: ['stdout'],
252+
encoding: 'utf8',
251253
};
252254
const binaryPath = '/mock-path/filename.mock';
253255
await adb.install(deviceId, binaryPath);

‎detox/src/devices/common/drivers/android/exec/BinaryExec.js

+8-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
// @ts-nocheck
2-
const spawn = require('child-process-promise').spawn;
3-
4-
const exec = require('../../../../../utils/childProcess').execWithRetriesAndLogs;
1+
const { execAsync, spawnAndLog } = require('../../../../../utils/childProcess');
52

63
class ExecCommand {
74
toString() {
@@ -27,15 +24,19 @@ class BinaryExec {
2724
}
2825

2926
async exec(command) {
30-
return (await exec(`"${this.binary}" ${command._getArgsString()}`)).stdout;
27+
return await execAsync(`"${this.binary}" ${command._getArgsString()}`);
3128
}
3229

3330
spawn(command, stdout, stderr) {
34-
return spawn(this.binary, command._getArgs(), { detached: true, stdio: ['ignore', stdout, stderr] });
31+
return spawnAndLog(this.binary, command._getArgs(), {
32+
detached: true,
33+
encoding: 'utf8',
34+
stdio: ['ignore', stdout, stderr]
35+
});
3536
}
3637
}
3738

3839
module.exports = {
3940
ExecCommand,
40-
BinaryExec,
41+
BinaryExec
4142
};

‎detox/src/devices/common/drivers/android/exec/BinaryExec.test.js

+12-18
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable node/no-extraneous-require */
12
describe('BinaryExec', () => {
23
const binaryPath = '/binary/mock';
34

@@ -6,16 +7,11 @@ describe('BinaryExec', () => {
67
let binaryExec;
78
beforeEach(() => {
89
jest.mock('../../../../../utils/childProcess', () => ({
9-
execWithRetriesAndLogs: jest.fn().mockResolvedValue({
10-
stdout: '',
11-
}),
10+
execAsync: jest.fn().mockResolvedValue(''),
11+
spawnAndLog: jest.fn()
1212
}));
13-
exec = require('../../../../../utils/childProcess').execWithRetriesAndLogs;
14-
15-
jest.mock('child-process-promise', () => ({
16-
spawn: jest.fn()
17-
}));
18-
spawn = require('child-process-promise').spawn;
13+
exec = require('../../../../../utils/childProcess').execAsync;
14+
spawn = require('../../../../../utils/childProcess').spawnAndLog;
1915

2016
const { BinaryExec } = require('./BinaryExec');
2117
binaryExec = new BinaryExec(binaryPath);
@@ -55,17 +51,15 @@ describe('BinaryExec', () => {
5551
expect(exec).toHaveBeenCalledWith(`"${binaryPath}" -mock -argz`);
5652
});
5753

58-
it('should return content of resolved promise\'s stdout', async () => {
59-
const execResult = {
60-
stdout: 'mock stdout content'
61-
};
54+
it('should return content of resolved promise', async () => {
55+
const execResult = 'mock stdout content';
6256
exec.mockResolvedValue(execResult);
6357

6458
const { ExecCommand } = require('./BinaryExec');
6559
const command = new ExecCommand();
6660
const result = await binaryExec.exec(command);
6761

68-
expect(result).toEqual(execResult.stdout);
62+
expect(result).toEqual(execResult);
6963
});
7064
});
7165

@@ -87,14 +81,14 @@ describe('BinaryExec', () => {
8781
expect(spawn).toHaveBeenCalledWith(binaryPath, commandArgs, expect.anything());
8882
});
8983

90-
it('should chain-return child-process-promise from spawn', async () => {
91-
const childProcessPromise = Promise.resolve('mock result');
92-
spawn.mockReturnValue(childProcessPromise);
84+
it('should chain-return spawn result', async () => {
85+
const spawnResult = Promise.resolve('mock result');
86+
spawn.mockReturnValue(spawnResult);
9387

9488
const command = anEmptyCommand();
9589

9690
const result = binaryExec.spawn(command);
97-
expect(result).toEqual(childProcessPromise);
91+
expect(result).toEqual(spawnResult);
9892
});
9993
});
10094
});

‎detox/src/devices/runtime/drivers/ios/SimulatorDriver.js

+6-7
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
// @ts-nocheck
22
const path = require('path');
33

4-
const exec = require('child-process-promise').exec;
54
const _ = require('lodash');
65

6+
77
const temporaryPath = require('../../../../artifacts/utils/temporaryPath');
88
const DetoxRuntimeError = require('../../../../errors/DetoxRuntimeError');
99
const XCUITestRunner = require('../../../../ios/XCUITestRunner');
1010
const { assertTraceDescription } = require('../../../../utils/assertArgument');
11+
const { execAsync } = require('../../../../utils/childProcess');
1112
const getAbsoluteBinaryPath = require('../../../../utils/getAbsoluteBinaryPath');
1213
const { actionDescription } = require('../../../../utils/invocationTraceDescriptions');
1314
const log = require('../../../../utils/logger').child({ cat: 'device' });
@@ -16,7 +17,6 @@ const traceInvocationCall = require('../../../../utils/traceInvocationCall').bin
1617

1718
const IosDriver = require('./IosDriver');
1819

19-
2020
/**
2121
* @typedef SimulatorDriverDeps { DeviceDriverDeps }
2222
* @property applesimutils { AppleSimUtils }
@@ -69,8 +69,7 @@ class SimulatorDriver extends IosDriver {
6969
async getBundleIdFromBinary(appPath) {
7070
appPath = getAbsoluteBinaryPath(appPath);
7171
try {
72-
const result = await exec(`/usr/libexec/PlistBuddy -c "Print CFBundleIdentifier" "${path.join(appPath, 'Info.plist')}"`);
73-
const bundleId = _.trim(result.stdout);
72+
const bundleId = await execAsync(`/usr/libexec/PlistBuddy -c "Print CFBundleIdentifier" "${path.join(appPath, 'Info.plist')}"`);
7473
if (_.isEmpty(bundleId)) {
7574
throw new Error();
7675
}
@@ -141,7 +140,7 @@ class SimulatorDriver extends IosDriver {
141140
const xcuitestRunner = new XCUITestRunner({ runtimeDevice: { id: this.getExternalId(), _bundleId } });
142141
let x = point?.x ?? 100;
143142
let y = point?.y ?? 100;
144-
let _pressDuration = pressDuration ? (pressDuration / 1000) : 1;
143+
let _pressDuration = pressDuration ? pressDuration / 1000 : 1;
145144
const traceDescription = actionDescription.longPress({ x, y }, _pressDuration);
146145
return this.withAction(xcuitestRunner, 'coordinateLongPress', traceDescription, x.toString(), y.toString(), _pressDuration.toString());
147146
}
@@ -210,7 +209,7 @@ class SimulatorDriver extends IosDriver {
210209
await this.emitter.emit('createExternalArtifact', {
211210
pluginId: 'screenshot',
212211
artifactName: screenshotName || path.basename(tempPath, '.png'),
213-
artifactPath: tempPath,
212+
artifactPath: tempPath
214213
});
215214

216215
return tempPath;
@@ -223,7 +222,7 @@ class SimulatorDriver extends IosDriver {
223222
await this.emitter.emit('createExternalArtifact', {
224223
pluginId: 'uiHierarchy',
225224
artifactName: artifactName,
226-
artifactPath: viewHierarchyURL,
225+
artifactPath: viewHierarchyURL
227226
});
228227

229228
return viewHierarchyURL;

‎detox/src/ios/XCUITestRunner.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
const { exec } = require('child-process-promise');
2-
31
const DetoxRuntimeError = require('../errors/DetoxRuntimeError');
2+
const { execAsync } = require('../utils/childProcess');
43
const environment = require('../utils/environment');
54
const log = require('../utils/logger').child({ cat: 'xcuitest-runner' });
65

@@ -37,7 +36,7 @@ class XCUITestRunner {
3736
`--predicate 'process == "DetoxXCUITestRunner-Runner" && subsystem == "com.wix.DetoxXCUITestRunner.xctrunner"'`);
3837

3938
try {
40-
return await exec(`TEST_RUNNER_PARAMS="${base64InvocationParams}" TEST_RUNNER_BUNDLE_ID="${this.runtimeDevice._bundleId}" xcodebuild ${flags.join(' ')}`);
39+
return await execAsync(`TEST_RUNNER_PARAMS="${base64InvocationParams}" TEST_RUNNER_BUNDLE_ID="${this.runtimeDevice._bundleId}" xcodebuild ${flags.join(' ')}`);
4140
} catch (e) {
4241
const stdout = e.stdout.toString();
4342
const innerError = this.findInnerError(stdout);

‎detox/src/ios/XCUITestRunner.test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
const XCUITestRunner = require('./XCUITestRunner');
22

3-
jest.mock('child-process-promise', () => {
3+
jest.mock('promisify-child-process', () => {
44
return {
55
exec: jest.fn(),
66
};
77
});
88

9-
const { exec } = jest.requireMock('child-process-promise');
9+
const { exec } = jest.requireMock('promisify-child-process');
1010
const environment = jest.requireMock('../utils/environment');
1111

1212
jest.mock('../utils/environment');

‎detox/src/utils/__snapshots__/assertArgument.test.js.snap

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,4 @@ exports[`assertUndefined should throw for "str" 1`] = `"0 expected to be undefin
4444

4545
exports[`assertUndefined should throw for {"key":"val"} 1`] = `"key expected to be undefined, but got val (string)"`;
4646

47-
exports[`assertUndefined should throw for 1 1`] = `"undefined is not iterable (cannot read property Symbol(Symbol.iterator))"`;
47+
exports[`assertUndefined should throw for 1 1`] = `"value expected to be undefined, but got 1 (number)"`;

‎detox/src/utils/assertArgument.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
const _ = require('lodash');
2+
13
const { DetoxInternalError, DetoxRuntimeError } = require('../errors');
24

35
function firstEntry(obj) {
4-
return Object.entries(obj)[0];
6+
return _.entries(obj)[0] || ['value', obj];
57
}
68

79
function assertType(expectedType) {

‎detox/src/utils/childProcess/exec.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
// @ts-nocheck
2-
const { exec } = require('child-process-promise');
32
const _ = require('lodash');
3+
const { exec } = require('promisify-child-process');
44

55
const DetoxRuntimeError = require('../../errors/DetoxRuntimeError');
66
const rootLogger = require('../logger').child({ cat: ['child-process', 'child-process-exec'] });
77
const retry = require('../retry');
88

99
const execsCounter = require('./opsCounter');
1010

11+
/**
12+
* Executes a command with retries and logs
13+
* @param {*} bin - command to execute
14+
* @param {*} options - options
15+
* @returns {Promise<import('promisify-child-process').Output>}
16+
*/
1117
async function execWithRetriesAndLogs(bin, options = {}) {
1218
const {
1319
retries = 9,

0 commit comments

Comments
 (0)
Please sign in to comment.