Skip to content

Commit fc4690c

Browse files
committed
feat(get-metadata): disable shell spawn on exec
1 parent 1dbb911 commit fc4690c

File tree

3 files changed

+48
-6
lines changed

3 files changed

+48
-6
lines changed

lib/dependencies/sub-process.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { spawn, SpawnOptions, spawnSync } from 'child_process';
2-
import { quoteAll } from 'shescape/stateless';
32

43
interface ProcessOptions {
54
cwd?: string;
@@ -8,7 +7,7 @@ interface ProcessOptions {
87

98
function makeSpawnOptions(options?: ProcessOptions) {
109
const spawnOptions: SpawnOptions = {
11-
shell: true,
10+
shell: false,
1211
env: { ...process.env },
1312
};
1413
if (options && options.cwd) {
@@ -39,12 +38,16 @@ export function execute(
3938
options?: ProcessOptions
4039
): Promise<string> {
4140
const spawnOptions = makeSpawnOptions(options);
42-
args = quoteAll(args, { flagProtection: false });
4341
return new Promise((resolve, reject) => {
4442
let stdout = '';
4543
let stderr = '';
4644

4745
const proc = spawn(command, args, spawnOptions);
46+
47+
proc.on('error', (error) => {
48+
reject(error);
49+
});
50+
4851
proc.stdout.on('data', (data) => {
4952
stdout = stdout + data;
5053
});
@@ -67,7 +70,6 @@ export function executeSync(
6770
options?: ProcessOptions
6871
) {
6972
const spawnOptions = makeSpawnOptions(options);
70-
args = quoteAll(args, { flagProtection: false });
7173

7274
return spawnSync(command, args, spawnOptions);
7375
}

test/test-utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ function createVenv(venvDir: string) {
103103
revert = deactivateVirtualenv();
104104
}
105105
try {
106-
let proc = subProcess.executeSync('python3 -m venv', [venvDir]);
106+
let proc = subProcess.executeSync('python3', ['-m', 'venv', venvDir]);
107107
if (proc.status !== 0) {
108108
console.error(proc.stdout.toString() + '\n' + proc.stderr.toString());
109109
throw new Error('Failed to create virtualenv in ' + venvDir);

test/unit/sub-process.spec.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { executeSync } from '../../lib/dependencies/sub-process';
1+
import { executeSync, execute } from '../../lib/dependencies/sub-process';
22

33
describe('Test sub-process.ts', () => {
44
it('test restoring proxy setting in executeSync()', async () => {
@@ -89,4 +89,44 @@ describe('Test sub-process.ts', () => {
8989
);
9090
expect(output.stdout.toString().trim()).toEqual(expectedNoProxy);
9191
});
92+
93+
describe('Security: Command injection protection', () => {
94+
it('should prevent command injection in executeSync()', () => {
95+
// Test that malicious command strings are treated as literal filenames (not executed)
96+
const maliciousCommand = 'python3; echo injected';
97+
const result = executeSync(maliciousCommand, ['--version']);
98+
99+
// Should fail with ENOENT because 'python3; echo injected' is not a valid executable
100+
expect(result.status).not.toBe(0);
101+
expect((result.error as any)?.code).toBe('ENOENT');
102+
});
103+
104+
it('should prevent command injection in execute()', async () => {
105+
// Test that malicious command strings are treated as literal filenames (not executed)
106+
const maliciousCommand = 'python3; whoami; echo injected';
107+
108+
try {
109+
await execute(maliciousCommand, ['--version']);
110+
fail('Expected execute() to reject with an error');
111+
} catch (error: any) {
112+
// Should fail with ENOENT because the malicious command is treated as a literal filename
113+
expect(error.code).toBe('ENOENT');
114+
expect(error.syscall).toBe(`spawn ${maliciousCommand}`);
115+
}
116+
});
117+
118+
it('should execute legitimate commands normally', async () => {
119+
// Verify that normal commands still work correctly
120+
const result = await execute('python3', ['--version']);
121+
expect(result).toMatch(/Python \d+\.\d+\.\d+/);
122+
});
123+
124+
it('should handle arguments with special characters safely', async () => {
125+
// Verify that special characters in arguments don't enable injection
126+
const result = await execute('python3', ['--version', '; echo injected']);
127+
// Should only show Python version, not execute the injected command
128+
expect(result).toMatch(/Python \d+\.\d+\.\d+/);
129+
expect(result).not.toMatch(/injected/);
130+
});
131+
});
92132
});

0 commit comments

Comments
 (0)