Fix: TUI-mode SSH connections could drop with "PTY allocation
request failed" / "shell request failed" runTuiSession awaited audit.create() (a Redis round-trip) and then accessibleHosts() (a directory API call) BEFORE calling runTui(), which is where the pty/shell/exec/subsystem listeners actually get attached to the session. The client sends its pty-req and shell requests immediately after opening the session -- if either await took long enough for those requests to arrive first, ssh2 auto-rejects any channel request with no listener (CHANNEL_FAILURE), which is exactly what OpenSSH reports as "PTY allocation request failed on channel 0" / "shell request failed on channel 0". The connection then just sat there, since nothing was left to drive it. runGrammar already has this exact fix (see its own comment); runTuiSession never got the equivalent treatment. Fixed the same way: register the session listeners synchronously, before any await, by having runTui take a Promise for the hosts list instead of the resolved list -- the shell handler awaits it internally once the client actually sends a shell request, which by construction happens only after the listener already exists. Verified: publickey auth against a real deployment succeeds (confirms the earlier ldaps:// fix holds), and the failure reproduces with the exact reported error strings for a bare `ssh user@host` (TUI/picker mode, no target) connection. Full suite: 50/50 passing, no regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -180,6 +180,23 @@ async function runGrammar(session, client, state) {
|
||||
}
|
||||
|
||||
async function runTuiSession(session, client, state) {
|
||||
// Register session listeners IMMEDIATELY, before any await — same fix,
|
||||
// same reason, as runGrammar above. The client sends pty-req and shell
|
||||
// requests right after opening the session; awaiting audit.create() and
|
||||
// accessibleHosts() first (both real round-trips: Redis, then the
|
||||
// directory API) left a window where those requests could arrive before
|
||||
// runTui had attached any listener for them, and ssh2 auto-rejects an
|
||||
// unlistened channel request with CHANNEL_FAILURE — surfacing to the
|
||||
// client as "PTY allocation request failed" / "shell request failed",
|
||||
// with the connection then just sitting there (nothing left to drive it).
|
||||
let resolveHosts, rejectHosts;
|
||||
const hostsPromise = new Promise((res, rej) => { resolveHosts = res; rejectHosts = rej; });
|
||||
// A silent catch so a rejection isn't "unhandled" if the client never
|
||||
// sends a shell request at all (exec-only) — runTui's own .catch() below
|
||||
// still runs independently when it does.
|
||||
hostsPromise.catch(() => {});
|
||||
const tuiPromise = runTui(session, state.uid, hostsPromise);
|
||||
|
||||
const record = await audit.create({ uid: state.uid, authMethod: state.authMethod, clientIp: state.clientIp, mode: 'tui' });
|
||||
|
||||
const finishFail = async (reason) => {
|
||||
@@ -189,10 +206,15 @@ async function runTuiSession(session, client, state) {
|
||||
};
|
||||
|
||||
let hosts;
|
||||
try { hosts = await accessibleHosts(state.user); }
|
||||
catch (_) { return finishFail('directory-unreachable'); }
|
||||
try {
|
||||
hosts = await accessibleHosts(state.user);
|
||||
resolveHosts(hosts);
|
||||
} catch (_) {
|
||||
rejectHosts(new Error('directory-unreachable'));
|
||||
return finishFail('directory-unreachable');
|
||||
}
|
||||
|
||||
const tui = await runTui(session, state.uid, hosts);
|
||||
const tui = await tuiPromise;
|
||||
if (!tui.host) return finishFail('cancelled');
|
||||
state.target = tui.host.slug;
|
||||
|
||||
@@ -253,7 +275,10 @@ function reasonMessage(reason) {
|
||||
|
||||
// Run the TUI picker over a shell channel; returns { host, channel, ptyInfo }.
|
||||
// host is null if the user quit. exec/subsystem in picker mode are rejected.
|
||||
function runTui(session, uid, hosts) {
|
||||
// Takes a Promise for the accessible-hosts list (not the resolved list)
|
||||
// so the caller can register these listeners before that lookup completes
|
||||
// — see the comment in runTuiSession for why that ordering matters.
|
||||
function runTui(session, uid, hostsPromise) {
|
||||
return new Promise((resolve) => {
|
||||
let ptyInfo = null;
|
||||
let settled = false;
|
||||
@@ -262,9 +287,14 @@ function runTui(session, uid, hosts) {
|
||||
session.on('pty', (accept, _reject, info) => { ptyInfo = info; accept && accept(); });
|
||||
session.on('shell', (accept) => {
|
||||
const channel = accept();
|
||||
pickHost(channel, uid, hosts).then((host) => {
|
||||
if (!host) { try { channel.write('\r\n Bye.\r\n'); channel.close(); } catch (_) {} }
|
||||
finish({ host, channel, ptyInfo });
|
||||
hostsPromise.then((hosts) => {
|
||||
pickHost(channel, uid, hosts).then((host) => {
|
||||
if (!host) { try { channel.write('\r\n Bye.\r\n'); channel.close(); } catch (_) {} }
|
||||
finish({ host, channel, ptyInfo });
|
||||
});
|
||||
}).catch(() => {
|
||||
try { channel.write('\r\n Could not reach the directory.\r\n'); channel.close(); } catch (_) {}
|
||||
finish({ host: null });
|
||||
});
|
||||
});
|
||||
session.on('exec', (accept) => {
|
||||
|
||||
Reference in New Issue
Block a user