fix: SCP/SFTP path resolution and dead chroot config (#186)

CHANGELOG.md

@@ -9,9 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 - Internal fork of `russh-sftp` as `crates/bssh-russh-sftp` with a `serde_bytes` performance fix for `SSH_FXP_WRITE` and `SSH_FXP_DATA` packets. The upstream serde derive routes `Vec<u8>` through `deserialize_seq` (byte-by-byte), accounting for ~42% of server CPU during 1 GiB SFTP uploads in `perf` profiling. Annotating the `data` fields with `#[serde(with = "serde_bytes")]` and implementing wire-compatible `serialize_bytes` on the SFTP `Serializer` routes through the existing bulk `deserialize_byte_buf`/`try_get_bytes` path. Measured impact on a CPU-bound host (Xeon Silver 4214): 1 GiB SFTP upload throughput improves from 74.8 MiB/s to 96.4 MiB/s (+29%), closing the gap to OpenSSH `sftp-server` from ~26% to ~5%.
+- `scp.root` configuration field. SCP transfers now honor a chroot setting separate from SFTP. When unset, SCP falls back to `sftp.root`, so a single top-level chroot setting governs both subsystems unless an admin explicitly wants them split.
 
 ### Changed
 - Switched the top-level `russh-sftp` dependency from crates.io `russh-sftp = "2.1.1"` to `russh-sftp = { package = "bssh-russh-sftp", version = "2.1.1", path = "crates/bssh-russh-sftp" }`. All existing `use russh_sftp::...` imports continue to work unchanged.
+- **Default file-transfer behavior is no longer chrooted to the user's home directory.** With `sftp.root`/`scp.root` unset (the default), absolute client paths are honored verbatim and relative paths resolve from the user's home directory, matching OpenSSH `sftp-server`/`scp` defaults. Deployments that intentionally want chroot-at-home-dir must now set `sftp.root: <home dir>` (or equivalent) explicitly. (#186)
+
+### Fixed
+- **bssh-server SCP/SFTP path doubling on absolute client paths** (#186). `ScpHandler::resolve_path` and `SftpHandler::resolve_path_static` previously re-rooted every absolute client path under the user's home directory, so `scp local user@host:/home/work/file.bin` wrote to `/home/work/home/work/file.bin` and `bssh upload local /abs/remote.bin` failed with `No such file`. The resolver now treats absolute client paths verbatim when no chroot is configured and rejects out-of-chroot absolute paths with `permission_denied` when one is. Path-traversal and symlink-escape protections continue to apply.
+- **SCP single-file destinations no longer have the source filename appended** (#186). `ScpHandler::receive_file` now consults `target_is_directory` (parsed from `-d`/`-r`) and the filesystem state of the resolved target. `scp local.bin user@host:/tmp/dest.bin` now writes to `/tmp/dest.bin` instead of `/tmp/dest.bin/local.bin`. Directory destinations (`/tmp/dir/`, existing directory, or `-d`/`-r` flag) keep the previous filename-appending behavior.
+- **Configured `sftp.root` is no longer dead code** (#186). The handler-construction sites in `SshHandler` previously hard-coded `user_info.home_dir` as the chroot root and ignored `config.sftp.root` entirely. Setting `sftp.root` in the YAML configuration now actually changes the SFTP chroot. The same plumbing now exists for `scp.root`.
+- **Chroot bypass via intermediate-directory symlink**. The chroot resolver previously checked only lexical containment for paths whose final component did not exist (typical for new-file creates and `mkdir`). A symlink inside the chroot pointing to a directory outside the chroot would let a client target `chroot/escape/newfile` and have `open(...)`/`create_dir(...)` follow the symlink, writing outside the chroot. Both `ScpHandler::resolve_path` and `SftpHandler::resolve_path_static` now canonicalize the closest existing ancestor of the target path and verify it stays inside the canonicalized chroot, blocking the parent-symlink escape. Found during PR #194 review.
 
 ## [2.1.2] - 2026-04-27
 

docs/architecture/server-configuration.md

@@ -140,12 +140,20 @@ shell:
 # SFTP subsystem configuration
 sftp:
   enabled: true                # Default: true
-  # Optional chroot directory
+  # Optional chroot directory.
+  # When unset (default), no chroot: absolute client paths are honored
+  # verbatim and relative paths resolve from the user's home directory.
+  # When set, clients are confined to this directory; absolute paths
+  # outside it are rejected with permission_denied.
   root: /data/sftp
 
 # SCP protocol configuration
 scp:
   enabled: true                # Default: true
+  # Optional chroot directory. Same semantics as sftp.root. When unset,
+  # falls back to sftp.root, so configure scp.root only when SCP and SFTP
+  # need separate chroots.
+  root: /data/scp
 
 # File transfer filtering
 filter:

docs/man/bssh-server.8

@@ -1,6 +1,6 @@
 .\" Manpage for bssh-server
 .\" Contact the maintainers to correct errors or typos.
-.TH BSSH-SERVER 8 "April 2026" "v2.1.2" "System Administration Commands"
+.TH BSSH-SERVER 8 "April 2026" "v2.1.3" "System Administration Commands"
 
 .SH NAME
 bssh-server \- Backend.AI SSH Server for container environments
@@ -173,10 +173,15 @@ Authentication methods and settings (methods, publickey, password)
 Shell execution settings (default, command_timeout, env)
 .TP
 .B sftp
-SFTP subsystem settings (enabled, root)
+SFTP subsystem settings (\fBenabled\fR, \fBroot\fR). \fBroot\fR sets a chroot
+directory that confines SFTP transfers. When unset (default), absolute client
+paths are honored verbatim and relative paths resolve from the user's home
+directory, matching OpenSSH \fBsftp-server\fR behavior.
 .TP
 .B scp
-SCP protocol settings (enabled)
+SCP protocol settings (\fBenabled\fR, \fBroot\fR). \fBroot\fR has the same
+semantics as \fBsftp.root\fR. When \fBscp.root\fR is unset, it falls back to
+\fBsftp.root\fR so a single setting governs both subsystems.
 .TP
 .B filter
 File transfer filtering (enabled, rules)
@@ -271,6 +276,11 @@ Enable IP allowlists in production to restrict access to trusted networks
 Configure rate limiting to prevent brute-force attacks
 .IP \(bu 2
 Enable audit logging for security monitoring and compliance
+.IP \(bu 2
+When \fBsftp.root\fR or \fBscp.root\fR is set, the chroot resolver
+canonicalizes the closest existing ancestor of the requested path and verifies
+it stays inside the configured root. This blocks attacks that route writes
+outside the chroot through intermediate-directory symlinks inside it.
 
 .SH EXIT STATUS
 .TP

docs/security.md

@@ -229,12 +229,39 @@ sftp:
 scp:
   enabled: false
 
-# Or enable with restrictions
+# Or enable with chroot. SCP falls back to sftp.root when scp.root is unset,
+# so a single setting governs both subsystems.
 sftp:
   enabled: true
-  root: /data/sftp  # Chroot to this directory
+  root: /data/sftp  # Confine SFTP and SCP to this directory.
+
+# Use scp.root only when SCP and SFTP need separate chroots:
+scp:
+  enabled: true
+  root: /data/scp
 ```
 
+**Chroot semantics.** When `root` is set:
+
+- Absolute client paths inside `root` are honored as-is. No path doubling.
+- Absolute client paths outside `root` are rejected with `permission_denied`.
+- Relative client paths resolve under `root`, with `..` clamped at the
+  chroot boundary.
+- The pseudo-root `/` (returned by `realpath`) maps back to the chroot
+  directory so interactive SFTP clients (`cd /`, `pwd`) still work.
+- Path-traversal and symlink-escape protections continue to apply,
+  including for paths whose final component does not exist yet: the closest
+  existing ancestor is canonicalized and verified to stay inside `root`.
+  This blocks intermediate-directory symlinks pointing outside the chroot.
+
+When `root` is unset (default since v2.1.3, per #186), the handler runs
+without chroot. Absolute paths are honored verbatim and relative paths
+resolve from the user's home directory, matching OpenSSH `sftp-server`.
+This is the recommended default for Backend.AI session containers and any
+deployment whose clients submit absolute filesystem paths (such as the
+WebUI's "Download SSH key / SCP example" snippet, or `bssh upload
+/abs/remote.bin`).
+
 ### File Transfer Filtering
 
 Block dangerous file types:

src/server/config/mod.rs

@@ -154,6 +154,25 @@ pub struct ServerConfig {
     #[serde(default = "default_true")]
     pub scp_enabled: bool,
 
+    /// Optional chroot directory for SFTP operations.
+    ///
+    /// When `None` (default), SFTP runs without chroot: absolute client paths
+    /// are used verbatim and relative paths resolve from the user's home
+    /// directory, matching OpenSSH `sftp-server` semantics.
+    ///
+    /// When set, SFTP clients are confined to this directory; absolute paths
+    /// outside it are rejected with `permission_denied`.
+    #[serde(default)]
+    pub sftp_root: Option<PathBuf>,
+
+    /// Optional chroot directory for SCP transfers.
+    ///
+    /// Has the same semantics as [`Self::sftp_root`]. When `None` and
+    /// `sftp_root` is set, SCP falls back to `sftp_root`. Configure both
+    /// fields only if SCP and SFTP need different chroots.
+    #[serde(default)]
+    pub scp_root: Option<PathBuf>,
+
     /// Time window for counting authentication attempts in seconds.
     ///
     /// Default: 300 (5 minutes)
@@ -293,6 +312,8 @@ impl Default for ServerConfig {
             password_auth: PasswordAuthConfigSerde::default(),
             exec: ExecConfig::default(),
             scp_enabled: true,
+            sftp_root: None,
+            scp_root: None,
             auth_window_secs: default_auth_window_secs(),
             ban_time_secs: default_ban_time_secs(),
             whitelist_ips: Vec::new(),
@@ -564,6 +585,25 @@ impl ServerConfigBuilder {
         self
     }
 
+    /// Set the SFTP chroot directory.
+    ///
+    /// When `None`, SFTP runs without chroot (OpenSSH-compatible default).
+    /// When set, SFTP clients are confined to this directory.
+    pub fn sftp_root(mut self, root: Option<PathBuf>) -> Self {
+        self.config.sftp_root = root;
+        self
+    }
+
+    /// Set the SCP chroot directory.
+    ///
+    /// When `None`, SCP falls back to [`sftp_root`](Self::sftp_root) if set,
+    /// otherwise runs without chroot. Set both fields only when SCP and SFTP
+    /// need different chroots.
+    pub fn scp_root(mut self, root: Option<PathBuf>) -> Self {
+        self.config.scp_root = root;
+        self
+    }
+
     /// Set the maximum sessions per user.
     pub fn max_sessions_per_user(mut self, max: usize) -> Self {
         self.config.max_sessions_per_user = max;
@@ -635,6 +675,10 @@ impl ServerFileConfig {
                 blocked_commands: Vec::new(),
             },
             scp_enabled: self.scp.enabled,
+            // SCP falls back to sftp.root when scp.root is unset so a single
+            // top-level chroot setting governs both subsystems.
+            scp_root: self.scp.root.or_else(|| self.sftp.root.clone()),
+            sftp_root: self.sftp.root,
             auth_window_secs: self.security.auth_window,
             ban_time_secs: self.security.ban_time,
             whitelist_ips: self.security.whitelist_ips,
@@ -701,6 +745,49 @@ mod tests {
         assert!(server_config.allow_password_auth);
     }
 
+    #[test]
+    fn sftp_root_threads_into_server_config() {
+        // Setting sftp.root should propagate to ServerConfig.sftp_root, and
+        // scp_root should fall back to it when scp.root is unset.
+        let mut file_config = ServerFileConfig::default();
+        file_config.server.host_keys = vec![PathBuf::from("/test/key")];
+        file_config.sftp.root = Some(PathBuf::from("/srv/sftp"));
+
+        let server_config = file_config.into_server_config();
+
+        assert_eq!(server_config.sftp_root, Some(PathBuf::from("/srv/sftp")));
+        assert_eq!(server_config.scp_root, Some(PathBuf::from("/srv/sftp")));
+    }
+
+    #[test]
+    fn scp_root_overrides_sftp_root_fallback() {
+        // When scp.root is explicitly set, it takes precedence over the
+        // sftp.root fallback so admins can split the two chroots.
+        let mut file_config = ServerFileConfig::default();
+        file_config.server.host_keys = vec![PathBuf::from("/test/key")];
+        file_config.sftp.root = Some(PathBuf::from("/srv/sftp"));
+        file_config.scp.root = Some(PathBuf::from("/srv/scp"));
+
+        let server_config = file_config.into_server_config();
+
+        assert_eq!(server_config.sftp_root, Some(PathBuf::from("/srv/sftp")));
+        assert_eq!(server_config.scp_root, Some(PathBuf::from("/srv/scp")));
+    }
+
+    #[test]
+    fn no_chroot_by_default() {
+        // The new default: both scp_root and sftp_root are None when no
+        // configuration is provided. This is the OpenSSH-compatible default
+        // documented for issue #186.
+        let mut file_config = ServerFileConfig::default();
+        file_config.server.host_keys = vec![PathBuf::from("/test/key")];
+
+        let server_config = file_config.into_server_config();
+
+        assert!(server_config.sftp_root.is_none());
+        assert!(server_config.scp_root.is_none());
+    }
+
     #[test]
     fn test_config_new() {
         let config = ServerConfig::new();

src/server/config/types.rs

@@ -247,10 +247,18 @@ pub struct SftpConfig {
     #[serde(default = "default_true")]
     pub enabled: bool,
 
-    /// Root directory for SFTP operations.
-    ///
-    /// If set, SFTP clients will be chrooted to this directory.
-    /// If None, users have access to the entire filesystem (subject to permissions).
+    /// Optional chroot directory for SFTP operations.
+    ///
+    /// When set, SFTP clients are confined to this directory:
+    /// - Absolute client paths inside `root` are honored as-is.
+    /// - Absolute client paths outside `root` are rejected with `permission_denied`.
+    /// - Relative client paths resolve under `root`.
+    /// - `..` traversal is clamped to `root`.
+    ///
+    /// When `None` (default), no chroot is applied. This matches OpenSSH
+    /// `sftp-server` behavior: absolute paths are used verbatim and relative
+    /// paths resolve from the user's home directory. Filesystem permissions
+    /// remain the only access control.
     pub root: Option<PathBuf>,
 }
 
@@ -263,6 +271,14 @@ pub struct ScpConfig {
     /// Default: true
     #[serde(default = "default_true")]
     pub enabled: bool,
+
+    /// Optional chroot directory for SCP transfers.
+    ///
+    /// Has the same semantics as [`SftpConfig::root`]. When `None`, SCP uses
+    /// `sftp.root` as a fallback so a single `root` setting controls both
+    /// subsystems. Set this explicitly only when SCP and SFTP need different
+    /// chroots.
+    pub root: Option<PathBuf>,
 }
 
 /// File transfer filtering configuration.
@@ -650,6 +666,7 @@ impl Default for ScpConfig {
     fn default() -> Self {
         Self {
             enabled: default_true(),
+            root: None,
         }
     }
 }

src/server/handler.rs

@@ -1001,11 +1001,14 @@ impl russh::server::Handler for SshHandler {
 
                 let handle_clone = handle.clone();
 
-                // Create SCP handler with user's home directory as root
+                // Honor the configured chroot. SCP falls back to sftp_root
+                // (already merged in into_server_config). When None, no
+                // chroot is applied, matching OpenSSH semantics.
                 let scp_handler = ScpHandler::from_command(
                     &scp_cmd,
                     user_info.clone(),
-                    Some(user_info.home_dir.clone()),
+                    self.config.scp_root.clone(),
+                    user_info.home_dir.clone(),
                 );
 
                 // Run SCP in a spawned task so the session loop can process incoming data
@@ -1335,8 +1338,13 @@ impl russh::server::Handler for SshHandler {
                     "Starting SFTP session"
                 );
 
-                // Create SFTP handler with user's home directory as root
-                let sftp_handler = SftpHandler::new(user_info.clone(), Some(user_info.home_dir));
+                // Honor the configured chroot. When `sftp_root` is None,
+                // run without chroot, matching OpenSSH `sftp-server` defaults.
+                let sftp_handler = SftpHandler::new(
+                    user_info.clone(),
+                    self.config.sftp_root.clone(),
+                    user_info.home_dir,
+                );
 
                 // Run SFTP server on the channel stream
                 russh_sftp::server::run(channel.into_stream(), sftp_handler).await;