Skip to content

Commit 3d3dbe2

Browse files
authored
Fix SSH session shell handling (#5)
Fixes critical SSH session bugs: - Adds missing SSHManager.requestShell() method - Routes SSH2 logs through structured logger to prevent stdio pollution - Fixes race condition in session state initialization All tests pass. Thank you @wen495033653!
1 parent 096dbcf commit 3d3dbe2

File tree

2 files changed

+33
-11
lines changed

2 files changed

+33
-11
lines changed

src/session-manager.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,12 @@ class SSHSession {
8282
// Wait for shell prompt
8383
await this.waitForPrompt();
8484

85+
// Allow context queries through standard execute flow
86+
this.state = SESSION_STATES.READY;
87+
8588
// Get initial working directory
8689
await this.updateContext();
8790

88-
this.state = SESSION_STATES.READY;
89-
9091
logger.info(`Session ${this.id} initialized`, {
9192
server: this.serverName,
9293
cwd: this.context.cwd
@@ -407,4 +408,4 @@ export default {
407408
closeServerSessions,
408409
cleanupSessions,
409410
SESSION_STATES
410-
};
411+
};

src/ssh-manager.js

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { promisify } from 'util';
44
import crypto from 'crypto';
55
import { isHostKnown, getCurrentHostKey, addHostKey, updateHostKey } from './ssh-key-manager.js';
66
import { configLoader } from './config-loader.js';
7+
import { logger } from './logger.js';
78

89
class SSHManager {
910
constructor(config) {
@@ -48,7 +49,7 @@ class SSHManager {
4849
},
4950
debug: (info) => {
5051
if (info.includes('Handshake') || info.includes('error')) {
51-
console.log('SSH2 Debug:', info);
52+
logger.debug('SSH2 Debug', { info });
5253
}
5354
}
5455
};
@@ -63,31 +64,35 @@ class SSHManager {
6364
if (isHostKnown(host, port)) {
6465
// For now, accept all known hosts
6566
// TODO: Implement proper fingerprint comparison once we understand SSH2's hash format
66-
console.log(`✅ Host ${host}:${port} is known, accepting connection`);
67+
logger.info('Host key verified', { host, port });
6768
return true;
6869
}
6970

7071
// Host is not known
71-
console.log(`🔑 New host detected: ${host}:${port}`);
72+
logger.info('New host detected', { host, port });
7273

7374
// If autoAcceptHostKey is enabled, accept and add the key
7475
if (this.autoAcceptHostKey) {
75-
console.log(`Auto-accepting and adding host key for ${host}:${port}`);
76+
logger.info('Auto-accept host key', { host, port });
7677
// Schedule key addition after connection
7778
setImmediate(async () => {
7879
try {
7980
await addHostKey(host, port);
80-
console.log(`✅ Host key added for ${host}:${port}`);
81+
logger.info('Host key added', { host, port });
8182
} catch (err) {
82-
console.warn(`Failed to add host key: ${err.message}`);
83+
logger.warn('Failed to add host key', {
84+
host,
85+
port,
86+
error: err.message
87+
});
8388
}
8489
});
8590
return true;
8691
}
8792

8893
// For backward compatibility, accept new hosts by default
8994
// In production, you might want to prompt the user or check a whitelist
90-
console.log(`Auto-accepting new host ${host}:${port} (set autoAcceptHostKey:false to reject)`);
95+
logger.warn('Auto-accepting new host', { host, port });
9196
return true;
9297
};
9398
}
@@ -237,6 +242,22 @@ class SSHManager {
237242
});
238243
}
239244

245+
async requestShell(options = {}) {
246+
if (!this.connected) {
247+
throw new Error('Not connected to SSH server');
248+
}
249+
250+
return new Promise((resolve, reject) => {
251+
this.client.shell(options, (err, stream) => {
252+
if (err) {
253+
reject(err);
254+
return;
255+
}
256+
resolve(stream);
257+
});
258+
});
259+
}
260+
240261
async getSFTP() {
241262
if (this.sftp) return this.sftp;
242263

@@ -432,4 +453,4 @@ class SSHManager {
432453
}
433454
}
434455

435-
export default SSHManager;
456+
export default SSHManager;

0 commit comments

Comments
 (0)