From 7a4dc44d6406ef6f7d897fe89e36b538f415af38 Mon Sep 17 00:00:00 2001
From: Sion Kang <siontama@gmail.com>
Date: Mon, 6 Jul 2026 19:01:53 +0900
Subject: [PATCH] fix(sftp): send channel EOF+CLOSE when the SFTP session ends
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The SFTP subsystem ran `russh_sftp::server::run()` which spawned the SFTP
request loop on a detached task and returned immediately, so the handler
never sent the server-side channel EOF/CLOSE after the session finished
(unlike the shell/exec/SCP paths, which do). Clients that block on the
SSH channel-close handshake — notably sshj/JSch (Cyberduck, PyCharm,
IntelliJ) — then wait for a CLOSE that never arrives and hang for ~30s
before timing out. OpenSSH `sftp`/FileZilla tolerate the missing CLOSE,
which is why the problem only showed up with sshj-based clients.

- bssh-russh-sftp: `run_with_config` no longer spawns internally; it runs
  the request loop inline and returns on client EOF, so the caller can
  observe completion and clean up the channel.
- handler: run the SFTP session on our own task and, when it ends, send
  `handle.eof()` + `handle.close()` (mirroring the shell/exec/SCP paths).
  Also close the channel on the user-info error paths.

Reproduced with an sshj 0.38 client (`canonicalize` + `stat` + `ls` +
close): before, the `SFTPClient.close()` timed out after 30s; after, the
session completes and closes in ~1s. OpenSSH `sftp` and paramiko continue
to work unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 crates/bssh-russh-sftp/src/server/mod.rs | 23 ++++++++++-------
 src/server/handler.rs                    | 33 +++++++++++++++++-------
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/crates/bssh-russh-sftp/src/server/mod.rs b/crates/bssh-russh-sftp/src/server/mod.rs
index a3e5a9ba..39d31828 100644
--- a/crates/bssh-russh-sftp/src/server/mod.rs
+++ b/crates/bssh-russh-sftp/src/server/mod.rs
@@ -104,21 +104,24 @@ where
     run_with_config(stream, handler, Config::default()).await
 }
 
-/// Run processing stream as SFTP with custom configuration
+/// Run processing stream as SFTP with custom configuration.
+///
+/// This runs the SFTP request loop inline and returns when the client closes
+/// the stream (EOF). Callers are responsible for spawning this onto a task if
+/// they need it to run concurrently, and for closing the underlying SSH
+/// channel once it returns.
 pub async fn run_with_config<S, H>(mut stream: S, mut handler: H, cfg: Config)
 where
     S: AsyncRead + AsyncWrite + Unpin + Send + 'static,
     H: Handler + Send + 'static,
 {
-    tokio::spawn(async move {
-        loop {
-            match process_handler(&mut stream, &mut handler, &cfg).await {
-                Err(Error::UnexpectedEof) => break,
-                Err(err) => warn!("{}", err),
-                Ok(_) => (),
-            }
+    loop {
+        match process_handler(&mut stream, &mut handler, &cfg).await {
+            Err(Error::UnexpectedEof) => break,
+            Err(err) => warn!("{}", err),
+            Ok(_) => (),
         }
+    }
 
-        debug!("sftp stream ended");
-    });
+    debug!("sftp stream ended");
 }
diff --git a/src/server/handler.rs b/src/server/handler.rs
index 694aa18a..e9906891 100644
--- a/src/server/handler.rs
+++ b/src/server/handler.rs
@@ -1310,6 +1310,9 @@ impl russh::server::Handler for SshHandler {
             // Clone what we need for the async block
             let auth_provider = Arc::clone(&self.auth_provider);
             let peer_addr = self.peer_addr;
+            let sftp_root = self.config.sftp_root.clone();
+            // Handle used to close the channel once the SFTP session ends.
+            let handle = session.handle();
 
             // Signal success before spawning the SFTP handler
             let _ = session.channel_success(channel_id);
@@ -1323,6 +1326,8 @@ impl russh::server::Handler for SshHandler {
                             user = %username,
                             "User not found after authentication for SFTP"
                         );
+                        let _ = handle.eof(channel_id).await;
+                        let _ = handle.close(channel_id).await;
                         return Ok(());
                     }
                     Err(e) => {
@@ -1331,6 +1336,8 @@ impl russh::server::Handler for SshHandler {
                             error = %e,
                             "Failed to get user info for SFTP"
                         );
+                        let _ = handle.eof(channel_id).await;
+                        let _ = handle.close(channel_id).await;
                         return Ok(());
                     }
                 };
@@ -1346,18 +1353,26 @@ impl russh::server::Handler for SshHandler {
                 // run without chroot, matching OpenSSH `sftp-server` defaults.
                 let sftp_handler = SftpHandler::new(
                     user_info.clone(),
-                    self.config.sftp_root.clone(),
+                    sftp_root,
                     user_info.home_dir,
                 );
 
-                // Run SFTP server on the channel stream
-                russh_sftp::server::run(channel.into_stream(), sftp_handler).await;
-
-                tracing::info!(
-                    user = %username,
-                    peer = ?peer_addr,
-                    "SFTP session ended"
-                );
+                // Run the SFTP session on a detached task so this handler
+                // returns promptly (keeping the russh event loop pumping
+                // channel data). When the session ends, send EOF + CLOSE so
+                // clients that block on the server's channel-close handshake
+                // (e.g. sshj/JSch, as used by Cyberduck/PyCharm) don't hang
+                // waiting ~30s for a close that never arrives.
+                tokio::spawn(async move {
+                    russh_sftp::server::run(channel.into_stream(), sftp_handler).await;
+                    let _ = handle.eof(channel_id).await;
+                    let _ = handle.close(channel_id).await;
+                    tracing::info!(
+                        user = %username,
+                        peer = ?peer_addr,
+                        "SFTP session ended"
+                    );
+                });
 
                 Ok(())
             }
