diff --git a/backend/src/workspaces/diagnostics.ts b/backend/src/workspaces/diagnostics.ts index 305a907f..b37cfb5b 100644 --- a/backend/src/workspaces/diagnostics.ts +++ b/backend/src/workspaces/diagnostics.ts @@ -160,8 +160,9 @@ export interface ConcreteDiagnosticAdapterDependencies { directProtocol?: DirectProtocolFactory; sshProcess?: SshProcessFactory; databaseClient?: DatabaseDiagnosticClientFactory; - sshSpawn?: (args: readonly string[]) => { kill(signal?: NodeJS.Signals): boolean }; + sshSpawn?: (args: readonly string[]) => { kill(signal?: NodeJS.Signals): boolean; once?(event: "error" | "exit", listener: (...args: any[]) => void): unknown }; reserveLoopbackPort?: () => Promise; + waitForSshReady?: (tunnel: LoopbackTunnel, signal: AbortSignal) => Promise; } /** @@ -231,6 +232,18 @@ async function reserveLoopbackPort(): Promise { } } +async function waitForLoopbackTunnel(tunnel: LoopbackTunnel, signal: AbortSignal): Promise { + while (!signal.aborted) { + try { + await connectTcp(tunnel.host, tunnel.port, signal); + return; + } catch { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + } + throw new Error("SSH tunnel readiness failed"); +} + /** * Concrete production adapters deliberately retain only probe metadata. Protocol failures and * response bodies are discarded at this boundary; callers receive fixed diagnostics instead. @@ -240,6 +253,7 @@ export function createConcreteDiagnosticAdapters( ): DiagnosticAdapters { const spawnSsh = dependencies.sshSpawn ?? ((args: readonly string[]) => spawn("ssh", [...args], { stdio: "ignore" })); const reserveSshPort = dependencies.reserveLoopbackPort ?? reserveLoopbackPort; + const waitForSshReady = dependencies.waitForSshReady ?? waitForLoopbackTunnel; const sshProcess = dependencies.sshProcess ?? { async start(request: SshTunnelRequest, args: readonly string[]) { const port = await reserveSshPort(); @@ -248,9 +262,29 @@ export function createConcreteDiagnosticAdapters( const child = spawnSsh(resolvedArgs); const abort = () => { child.kill("SIGTERM"); }; request.signal.addEventListener("abort", abort, { once: true }); + const tunnel = { host: "127.0.0.1" as const, port }; + try { + await withTimeout(request.timeoutMs, async (signal) => { + await Promise.race([ + waitForSshReady(tunnel, signal), + new Promise((_resolve, reject) => { + child.once?.("error", () => reject(new Error("SSH tunnel readiness failed"))); + child.once?.("exit", () => reject(new Error("SSH tunnel readiness failed"))); + }), + ]); + }); + } catch (error) { + request.signal.removeEventListener("abort", abort); + child.kill("SIGTERM"); + throw error; + } return { - tunnel: { host: "127.0.0.1" as const, port }, - async close() { request.signal.removeEventListener("abort", abort); child.kill("SIGTERM"); }, + tunnel, + async close() { + request.signal.removeEventListener("abort", abort); + child.kill("SIGTERM"); + if (child.once) await withTimeout(request.timeoutMs, () => new Promise((resolve) => child.once?.("exit", resolve))).catch(() => undefined); + }, }; }, }; @@ -332,7 +366,7 @@ export function createConcreteDiagnosticAdapters( throw new Error("SSH probe failed"); } const args = [ - "-N", "-o", "BatchMode=yes", "-o", "StrictHostKeyChecking=yes", + "-N", "-o", "BatchMode=yes", "-o", "ExitOnForwardFailure=yes", "-o", "StrictHostKeyChecking=yes", "-o", `UserKnownHostsFile=${request.knownHostsFile}`, "-i", request.privateKeyFile, "-p", String(request.sshPort), "-L", `127.0.0.1:0:${request.targetHost}:${request.targetPort}`, `${request.sshUser}@${request.sshHost}`, diff --git a/backend/test/workspaces-diagnostics.test.ts b/backend/test/workspaces-diagnostics.test.ts index 5d46c6a3..11673102 100644 --- a/backend/test/workspaces-diagnostics.test.ts +++ b/backend/test/workspaces-diagnostics.test.ts @@ -333,7 +333,7 @@ test("provides a default bounded SSH factory through injected spawn and loopback const kill = vi.fn(() => true); const sshSpawn = vi.fn(() => ({ kill })); try { - const adapter = createConcreteDiagnosticAdapters({ sshSpawn, reserveLoopbackPort: async () => 45432 } as any); + const adapter = createConcreteDiagnosticAdapters({ sshSpawn, reserveLoopbackPort: async () => 45432, waitForSshReady: async () => undefined } as any); await adapter.withSshTunnel({ sshHost: "bastion.example.test", sshPort: 22, sshUser: "tunnel", privateKeyFile, knownHostsFile: "/run/secrets/known-hosts", targetHost: "dwh.internal", targetPort: 5432, localHost: "127.0.0.1", localPort: 0, timeoutMs: 5000, signal: new AbortController().signal }, async () => undefined); expect(sshSpawn).toHaveBeenCalledWith(expect.arrayContaining([ "StrictHostKeyChecking=yes", "UserKnownHostsFile=/run/secrets/known-hosts", "-L", "127.0.0.1:45432:dwh.internal:5432", @@ -344,6 +344,28 @@ test("provides a default bounded SSH factory through injected spawn and loopback } }); +test("waits for SSH readiness before probing and includes ExitOnForwardFailure", async () => { + const directory = await mkdtemp(join(tmpdir(), "thothii-diagnostic-")); + const privateKeyFile = join(directory, "ssh-key"); + await writeFile(privateKeyFile, "test-key\n", { mode: 0o600 }); + let releaseReady: (() => void) | undefined; + const ready = new Promise((resolve) => { releaseReady = resolve; }); + const probe = vi.fn(async () => undefined); + const sshSpawn = vi.fn(() => ({ kill: vi.fn(() => true) })); + try { + const adapter = createConcreteDiagnosticAdapters({ sshSpawn, reserveLoopbackPort: async () => 45432, waitForSshReady: async () => await ready } as any); + const running = adapter.withSshTunnel({ sshHost: "bastion.example.test", sshPort: 22, sshUser: "tunnel", privateKeyFile, knownHostsFile: "/run/secrets/known-hosts", targetHost: "dwh.internal", targetPort: 5432, localHost: "127.0.0.1", localPort: 0, timeoutMs: 5000, signal: new AbortController().signal }, probe); + await Promise.resolve(); + expect(probe).not.toHaveBeenCalled(); + releaseReady?.(); + await running; + expect(sshSpawn).toHaveBeenCalledWith(expect.arrayContaining(["ExitOnForwardFailure=yes"])); + expect(probe).toHaveBeenCalledOnce(); + } finally { + await rm(directory, { recursive: true, force: true }); + } +}); + test("requires a matching embedding model vector and removes its unique write probe", async () => { const adapters = successfulAdapters();