This commit is contained in:
@@ -13,6 +13,11 @@ class StubChannel extends EventEmitter {
|
||||
}
|
||||
}
|
||||
|
||||
class StubClient extends EventEmitter {
|
||||
ended = false;
|
||||
end(): void { this.ended = true; this.emit('close'); }
|
||||
}
|
||||
|
||||
function mkAudit() {
|
||||
return {
|
||||
beginAndComplete: vi.fn(),
|
||||
@@ -21,7 +26,7 @@ function mkAudit() {
|
||||
};
|
||||
}
|
||||
|
||||
function mkSession(channel: StubChannel) {
|
||||
function mkSession(channel: StubChannel, client: StubClient = new StubClient()) {
|
||||
const audit = mkAudit();
|
||||
const session = new ConsoleSession({
|
||||
localTaskId: 't1',
|
||||
@@ -32,9 +37,10 @@ function mkSession(channel: StubChannel) {
|
||||
rows: 24,
|
||||
scrollbackCap: 1024,
|
||||
channel: channel as any,
|
||||
client: client as any,
|
||||
auditRepo: audit as any,
|
||||
});
|
||||
return { session, audit };
|
||||
return { session, audit, client };
|
||||
}
|
||||
|
||||
describe('ConsoleSession', () => {
|
||||
@@ -131,18 +137,43 @@ describe('ConsoleSession', () => {
|
||||
expect(ch.windowChanges).toEqual([{ rows: 40, cols: 100 }]);
|
||||
});
|
||||
|
||||
it('close() is idempotent and records audit', async () => {
|
||||
it('close() is idempotent, ends the client, and records audit', async () => {
|
||||
const ch = new StubChannel();
|
||||
const { session, audit } = mkSession(ch);
|
||||
const { session, audit, client } = mkSession(ch);
|
||||
await session.close('idle_timeout');
|
||||
await session.close('idle_timeout');
|
||||
expect(ch.ended).toBe(true);
|
||||
expect(client.ended).toBe(true);
|
||||
expect(audit.beginAndComplete).toHaveBeenCalledTimes(1);
|
||||
const call = audit.beginAndComplete.mock.calls[0]![0];
|
||||
expect(call.action).toBe('ssh.console.close');
|
||||
expect(call.detail.reason).toBe('idle_timeout');
|
||||
});
|
||||
|
||||
it('client "error" (ECONNRESET) tears the session down without throwing', async () => {
|
||||
const ch = new StubChannel();
|
||||
const { session, audit, client } = mkSession(ch);
|
||||
// Regression for #407: an unhandled ssh2 Client 'error' event crashes
|
||||
// the whole Node process. ConsoleSession must own a listener so a
|
||||
// dropped transport closes only this session.
|
||||
expect(() => client.emit('error', new Error('read ECONNRESET'))).not.toThrow();
|
||||
// close() runs on the next tick via the catch-less promise chain.
|
||||
await Promise.resolve();
|
||||
expect(session.isClosed).toBe(true);
|
||||
expect(audit.beginAndComplete).toHaveBeenCalledTimes(1);
|
||||
expect(audit.beginAndComplete.mock.calls[0]![0].detail.reason).toBe('host_disconnect');
|
||||
});
|
||||
|
||||
it('client "close" tears the session down once', async () => {
|
||||
const ch = new StubChannel();
|
||||
const { session, audit, client } = mkSession(ch);
|
||||
client.emit('close');
|
||||
await Promise.resolve();
|
||||
expect(session.isClosed).toBe(true);
|
||||
expect(audit.beginAndComplete).toHaveBeenCalledTimes(1);
|
||||
expect(audit.beginAndComplete.mock.calls[0]![0].detail.reason).toBe('host_disconnect');
|
||||
});
|
||||
|
||||
it('scrollback caps at scrollbackCap', () => {
|
||||
const ch = new StubChannel();
|
||||
const { session } = mkSession(ch);
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { createRequire } from 'node:module';
|
||||
import type { Terminal as HeadlessTerminalType } from '@xterm/headless';
|
||||
import type { ClientChannel } from 'ssh2';
|
||||
import type { Client, ClientChannel } from 'ssh2';
|
||||
import { ByteRingBuffer } from './ring-buffer.js';
|
||||
/** Where an input chunk came from — drives audit `source` + back-pressure label. */
|
||||
export type InputSource = 'human' | 'ai';
|
||||
@@ -26,6 +26,13 @@ export interface ConsoleSessionArgs {
|
||||
rows: number;
|
||||
scrollbackCap: number;
|
||||
channel: ClientChannel;
|
||||
/**
|
||||
* The ssh2 Client backing `channel`. The session owns it: it ends the
|
||||
* client on close() (so the underlying connection/socket doesn't leak)
|
||||
* and reacts to client-level 'error'/'close' so a dropped transport tears
|
||||
* the session down instead of leaving a half-dead session registered.
|
||||
*/
|
||||
client: Client;
|
||||
auditRepo: SshAuditRepo;
|
||||
}
|
||||
|
||||
@@ -108,6 +115,7 @@ export class ConsoleSession {
|
||||
rows: number;
|
||||
|
||||
private readonly channel: ClientChannel;
|
||||
private readonly client: Client;
|
||||
private readonly headless: HeadlessTerminal;
|
||||
private readonly scrollback: ByteRingBuffer;
|
||||
private readonly auditRepo: SshAuditRepo;
|
||||
@@ -131,6 +139,7 @@ export class ConsoleSession {
|
||||
this.cols = args.cols;
|
||||
this.rows = args.rows;
|
||||
this.channel = args.channel;
|
||||
this.client = args.client;
|
||||
this.scrollback = new ByteRingBuffer(args.scrollbackCap);
|
||||
this.auditRepo = args.auditRepo;
|
||||
this.headless = new HeadlessTerminal({
|
||||
@@ -144,13 +153,29 @@ export class ConsoleSession {
|
||||
});
|
||||
|
||||
this.channel.on('data', (data: Buffer) => this.handleOutput(data));
|
||||
this.channel.on('close', () => {
|
||||
if (!this.closing) {
|
||||
this.close('host_disconnect').catch((e) =>
|
||||
logger.warn(`[console-session] close error: ${(e as Error).message}`),
|
||||
);
|
||||
}
|
||||
this.channel.on('close', () => this.onTransportDown());
|
||||
|
||||
// The Client (connection) can fail independently of the channel:
|
||||
// ssh2 re-emits transport-level socket errors (ECONNRESET on idle
|
||||
// timeout, network drop) as an 'error' on the Client. Without a
|
||||
// listener Node treats an 'error' event as fatal and kills the whole
|
||||
// process — this handler keeps a dropped connection to a single
|
||||
// session-scoped teardown. 'close' covers a graceful peer disconnect.
|
||||
this.client.on('error', (e: Error) => {
|
||||
logger.warn(
|
||||
`[console-session] client error task=${this.localTaskId}: ${(e as Error).message}`,
|
||||
);
|
||||
this.onTransportDown();
|
||||
});
|
||||
this.client.on('close', () => this.onTransportDown());
|
||||
}
|
||||
|
||||
/** Tear the session down once when the underlying SSH transport drops. */
|
||||
private onTransportDown(): void {
|
||||
if (this.closing) return;
|
||||
this.close('host_disconnect').catch((e) =>
|
||||
logger.warn(`[console-session] close error: ${(e as Error).message}`),
|
||||
);
|
||||
}
|
||||
|
||||
get lastActivityAt(): number {
|
||||
@@ -275,6 +300,15 @@ export class ConsoleSession {
|
||||
} catch {
|
||||
/* already gone */
|
||||
}
|
||||
try {
|
||||
// End the owning Client too — channel.end() only closes the shell
|
||||
// channel, the TCP connection stays up until the client is ended.
|
||||
// Guarded by `closing` above so the resulting 'close' event is a
|
||||
// no-op in onTransportDown.
|
||||
this.client.end();
|
||||
} catch {
|
||||
/* already gone */
|
||||
}
|
||||
try {
|
||||
this.headless.dispose();
|
||||
} catch {
|
||||
|
||||
+35
-8
@@ -219,15 +219,42 @@ async function openClient(
|
||||
const settle = (err: Error | null) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
client.removeAllListeners('ready');
|
||||
client.removeAllListeners('error');
|
||||
client.removeAllListeners('close');
|
||||
if (err) reject(err);
|
||||
else resolve(client);
|
||||
// Detach the handshake-only listeners with the exact references
|
||||
// (removeAllListeners would also strip the permanent error handler).
|
||||
client.removeListener('ready', onReady);
|
||||
client.removeListener('close', onClose);
|
||||
if (err) {
|
||||
// Failed handshake: the caller destroys the socket and never uses
|
||||
// this client, so drop the error handler too.
|
||||
client.removeListener('error', onError);
|
||||
reject(err);
|
||||
} else {
|
||||
// Success: keep `onError` attached. The client can outlive this
|
||||
// call (an interactive console keeps it alive for the whole
|
||||
// session), and ssh2 re-emits transport-level failures
|
||||
// (ECONNRESET on idle timeout, network drop) as an 'error' event
|
||||
// on the Client. A Node 'error' event with no listener is fatal to
|
||||
// the entire process — this permanent handler is the safety net
|
||||
// that turns a dropped connection into a log line instead of a
|
||||
// crash. The live-client owner (ConsoleSession / exec finally)
|
||||
// drives the actual teardown.
|
||||
resolve(client);
|
||||
}
|
||||
};
|
||||
client.once('ready', () => settle(null));
|
||||
client.once('error', (err: Error) => settle(err));
|
||||
client.once('close', () => settle(new Error('connection_closed_during_handshake')));
|
||||
const onReady = () => settle(null);
|
||||
const onClose = () => settle(new Error('connection_closed_during_handshake'));
|
||||
const onError = (err: Error) => {
|
||||
if (!settled) {
|
||||
settle(err);
|
||||
} else {
|
||||
logger.warn(
|
||||
`[ssh:session] client error after handshake conn=${connection.id}: ${sanitizeError(err).message}`,
|
||||
);
|
||||
}
|
||||
};
|
||||
client.on('error', onError);
|
||||
client.once('ready', onReady);
|
||||
client.once('close', onClose);
|
||||
try {
|
||||
client.connect(config);
|
||||
} catch (e) {
|
||||
|
||||
Reference in New Issue
Block a user