Skip to content

Commit 18c269f

Browse files
dorlugasigalCopilot
andcommitted
fix(cli): skip non-shell parent processes in shell detection
When run via npx, ps -o comm= returns the npm command (e.g. 'npm exec termbeam@latest') instead of a real shell. This was passed directly to node-pty, causing posix_spawnp to fail. Now validates the detected parent process looks like a real shell (no spaces, not npm/node) and falls back to $SHELL or /bin/sh. Also adds e2e tests for CLI banner output and invalid shell rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 45ba4dc commit 18c269f

2 files changed

Lines changed: 145 additions & 14 deletions

File tree

src/cli.js

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,10 @@ function getWindowsAncestors(startPid, maxDepth = 4) {
7676
if (cols.length <= Math.max(nameIdx, pidIdx, ppidIdx)) continue;
7777
const pid = parseInt(cols[pidIdx], 10);
7878
if (Number.isFinite(pid)) {
79-
processes.set(pid, { name: cols[nameIdx].trim().toLowerCase(), ppid: parseInt(cols[ppidIdx], 10) });
79+
processes.set(pid, {
80+
name: cols[nameIdx].trim().toLowerCase(),
81+
ppid: parseInt(cols[ppidIdx], 10),
82+
});
8083
}
8184
}
8285

@@ -137,8 +140,14 @@ function getDefaultShell() {
137140
const comm = result.trim();
138141
if (comm) {
139142
const shell = comm.startsWith('-') ? comm.slice(1) : comm;
140-
log.debug(`Detected parent shell: ${shell}`);
141-
return shell;
143+
log.debug(`Detected parent process: ${shell}`);
144+
// Validate it looks like a real shell (single token, no spaces)
145+
// When run via npx, comm can be "npm exec ..." which is not a shell
146+
if (!shell.includes(' ') && !shell.startsWith('npm') && !shell.startsWith('node')) {
147+
log.debug(`Using detected shell: ${shell}`);
148+
return shell;
149+
}
150+
log.debug(`Parent process "${shell}" is not a shell, falling back`);
142151
}
143152
} catch (err) {
144153
log.debug(`Could not detect parent shell: ${err.message}`);
@@ -157,10 +166,16 @@ function parseArgs() {
157166
// Resolve log level early (env + args) so shell detection logs are visible
158167
let logLevel = process.env.TERMBEAM_LOG_LEVEL || 'info';
159168
for (const arg of process.argv.slice(2)) {
160-
if (arg.startsWith('--log-level=')) { logLevel = arg.split('=')[1]; break; }
169+
if (arg.startsWith('--log-level=')) {
170+
logLevel = arg.split('=')[1];
171+
break;
172+
}
161173
}
162174
for (let i = 2; i < process.argv.length; i++) {
163-
if (process.argv[i] === '--log-level' && process.argv[i + 1]) { logLevel = process.argv[i + 1]; break; }
175+
if (process.argv[i] === '--log-level' && process.argv[i + 1]) {
176+
logLevel = process.argv[i + 1];
177+
break;
178+
}
164179
}
165180
log.setLevel(logLevel);
166181

@@ -227,7 +242,19 @@ function parseArgs() {
227242
const { getVersion } = require('./version');
228243
const version = getVersion();
229244

230-
return { port, host, password, useTunnel, persistedTunnel, shell, shellArgs, cwd, defaultShell, version, logLevel };
245+
return {
246+
port,
247+
host,
248+
password,
249+
useTunnel,
250+
persistedTunnel,
251+
shell,
252+
shellArgs,
253+
cwd,
254+
defaultShell,
255+
version,
256+
logLevel,
257+
};
231258
}
232259

233260
module.exports = { parseArgs, printHelp };

test/integration.test.js

Lines changed: 112 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
const { describe, it, after } = require('node:test');
22
const assert = require('node:assert');
33
const http = require('http');
4+
const { spawn } = require('child_process');
5+
const path = require('path');
46
const WebSocket = require('ws');
57
const { createTermBeamServer } = require('../src/server');
68

@@ -59,8 +61,14 @@ function waitForOpen(ws, timeout = 5000) {
5961
return new Promise((resolve, reject) => {
6062
if (ws.readyState === WebSocket.OPEN) return resolve();
6163
const timer = setTimeout(() => reject(new Error('WebSocket open timeout')), timeout);
62-
ws.on('open', () => { clearTimeout(timer); resolve(); });
63-
ws.on('error', (err) => { clearTimeout(timer); reject(err); });
64+
ws.on('open', () => {
65+
clearTimeout(timer);
66+
resolve();
67+
});
68+
ws.on('error', (err) => {
69+
clearTimeout(timer);
70+
reject(err);
71+
});
6472
});
6573
}
6674

@@ -117,7 +125,10 @@ describe('Integration', () => {
117125
port: inst.port,
118126
path: '/api/auth',
119127
method: 'POST',
120-
headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(authBody) },
128+
headers: {
129+
'Content-Type': 'application/json',
130+
'Content-Length': Buffer.byteLength(authBody),
131+
},
121132
},
122133
authBody,
123134
);
@@ -176,7 +187,11 @@ describe('Integration', () => {
176187

177188
// Send input and wait for output containing the echo marker
178189
const marker = `helloTB${Date.now()}`;
179-
const outputPromise = waitForMessage(ws, (m) => m.type === 'output' && m.data.includes(marker), 15000);
190+
const outputPromise = waitForMessage(
191+
ws,
192+
(m) => m.type === 'output' && m.data.includes(marker),
193+
15000,
194+
);
180195
ws.send(JSON.stringify({ type: 'input', data: `echo ${marker}\r` }));
181196
const outputMsg = await outputPromise;
182197
assert.ok(outputMsg.data.includes(marker), 'Output should contain the echoed marker');
@@ -235,7 +250,10 @@ describe('Integration', () => {
235250
port: inst.port,
236251
path: '/api/sessions',
237252
method: 'POST',
238-
headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(createBody) },
253+
headers: {
254+
'Content-Type': 'application/json',
255+
'Content-Length': Buffer.byteLength(createBody),
256+
},
239257
},
240258
createBody,
241259
);
@@ -277,7 +295,10 @@ describe('Integration', () => {
277295
resolve();
278296
}
279297
}, 100);
280-
setTimeout(() => { clearInterval(poll); resolve(); }, 5000);
298+
setTimeout(() => {
299+
clearInterval(poll);
300+
resolve();
301+
}, 5000);
281302
});
282303

283304
// GET /api/sessions should only have the default session
@@ -334,7 +355,10 @@ describe('Integration', () => {
334355
port: inst.port,
335356
path: '/api/auth',
336357
method: 'POST',
337-
headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(wrongBody) },
358+
headers: {
359+
'Content-Type': 'application/json',
360+
'Content-Length': Buffer.byteLength(wrongBody),
361+
},
338362
},
339363
wrongBody,
340364
);
@@ -348,11 +372,91 @@ describe('Integration', () => {
348372
port: inst.port,
349373
path: '/api/auth',
350374
method: 'POST',
351-
headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(wrongBody) },
375+
headers: {
376+
'Content-Type': 'application/json',
377+
'Content-Length': Buffer.byteLength(wrongBody),
378+
},
352379
},
353380
wrongBody,
354381
);
355382
assert.strictEqual(res6.statusCode, 429, '6th attempt should be rate limited');
356383
});
357384
});
385+
386+
describe('CLI entry point produces output (npx simulation)', () => {
387+
it('should print the banner when invoked via a wrapper script', async () => {
388+
const entryPoint = path.resolve(__dirname, '..', 'bin', 'termbeam.js');
389+
const output = await new Promise((resolve, reject) => {
390+
let buf = '';
391+
const child = spawn(process.execPath, [entryPoint, '--no-tunnel', '--no-password'], {
392+
env: { ...process.env, TERMBEAM_LOG_LEVEL: 'error', PORT: '0' },
393+
stdio: ['ignore', 'pipe', 'pipe'],
394+
});
395+
child.stdout.on('data', (d) => {
396+
buf += d;
397+
});
398+
child.stderr.on('data', (d) => {
399+
buf += d;
400+
});
401+
const timer = setTimeout(() => {
402+
child.kill('SIGTERM');
403+
resolve(buf);
404+
}, 5000);
405+
child.on('error', (err) => {
406+
clearTimeout(timer);
407+
reject(err);
408+
});
409+
child.on('exit', () => {
410+
clearTimeout(timer);
411+
resolve(buf);
412+
});
413+
});
414+
assert.ok(
415+
output.includes('Beam your terminal') || output.includes('TERMBEAM'),
416+
'Should print banner, got: ' + output.slice(0, 200),
417+
);
418+
});
419+
});
420+
421+
describe('Server rejects invalid shell and falls back gracefully', () => {
422+
let inst;
423+
after(() => inst?.shutdown());
424+
425+
it('should start even when defaultShell is an invalid process name', async () => {
426+
// Simulate the npx bug: defaultShell is garbage but shell is a real shell
427+
inst = await startServer({
428+
defaultShell: 'npm exec termbeam@latest --log-level=debug',
429+
});
430+
const res = await httpRequest({
431+
hostname: '127.0.0.1',
432+
port: inst.port,
433+
path: '/api/sessions',
434+
method: 'GET',
435+
});
436+
assert.strictEqual(res.statusCode, 200);
437+
const sessions = JSON.parse(res.data);
438+
assert.ok(sessions.length >= 1, 'Should have at least one session');
439+
});
440+
441+
it('POST /api/sessions with invalid shell should return 400', async () => {
442+
if (!inst) inst = await startServer();
443+
const body = JSON.stringify({ name: 'bad', shell: 'npm exec termbeam' });
444+
const res = await httpRequest(
445+
{
446+
hostname: '127.0.0.1',
447+
port: inst.port,
448+
path: '/api/sessions',
449+
method: 'POST',
450+
headers: {
451+
'Content-Type': 'application/json',
452+
'Content-Length': Buffer.byteLength(body),
453+
},
454+
},
455+
body,
456+
);
457+
assert.strictEqual(res.statusCode, 400);
458+
const data = JSON.parse(res.data);
459+
assert.strictEqual(data.error, 'Invalid shell');
460+
});
461+
});
358462
});

0 commit comments

Comments
 (0)