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

[inureyes]

Summary

Fixes the three defects in `bssh-server`'s file-transfer subsystems that
made SCP and SFTP uploads with absolute destination paths fail, blocking
adoption of `bssh-server` as a drop-in OpenSSH replacement in Backend.AI
session containers. A chroot-bypass security vulnerability found during
review is also fixed.

1. Path doubling. `ScpHandler::resolve_path` and
   `SftpHandler::resolve_path_static` unconditionally re-rooted every
   absolute client path under the user's home directory, so
   `scp local user@host:/home/work/file.bin` produced
   `/home/work/home/work/file.bin`. The same bug caused `bssh upload`
   to fail with `No such file`. Both resolvers now carry an
   `Option` chroot root and a separate `cwd` for relative
   paths. With no chroot (the new default), absolute paths are honored
   verbatim and relative paths resolve from the user's home directory,
   matching OpenSSH `sftp-server`/`scp`. With chroot, absolute paths
   inside the chroot are honored verbatim (no doubling); absolute paths
   outside are rejected with `permission_denied`. Plain `/` keeps
   mapping to the chroot directory so the `realpath` round-trip used
   by interactive SFTP clients still works.
2. SCP filename appending. `receive_file` always appended the source
   filename to the resolved target, so
   `scp local.bin host:/tmp/dest.bin` wrote to `/tmp/dest.bin/local.bin`.
   The handler now consults `target_is_directory` (parsed from `-d`/`-r`)
   and the filesystem state of the resolved target. Single-file
   destinations write directly to the resolved path; directory
   destinations (existing directory, `-d`/`-r` flag, or recursive
   descent) keep the filename-appending behavior.
3. Dead-code chroot config. `SshHandler` previously hard-coded
   `user_info.home_dir` as the chroot root and ignored
   `config.sftp.root`. Both handler-construction sites now read
   `config.sftp_root`/`config.scp_root` and thread them through.
4. Chroot bypass via intermediate-directory symlink (security fix).
   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 root, blocking parent-symlink escapes.

Default behavior change (migration note)

With `sftp.root`/`scp.root` unset (the default), the server no longer
applies an implicit chroot at the user's home directory. Deployments
that intentionally wanted that confinement must now set the field
explicitly, e.g.

```yaml
sftp:
root: /home/work # or whatever the desired chroot is
```

This matches OpenSSH defaults and is the recommended setup for
Backend.AI session containers per the issue. The `scp.root` field is
new and falls back to `sftp.root` when unset, so a single top-level
chroot setting governs both subsystems unless an admin explicitly
splits them.

Design choice: `scp.root` falls back to `sftp.root`

The issue suggested either adding `scp.root` or sharing `sftp.root`. I
went with both: `ScpConfig.root: Option` exists and, when
unset, the loader falls back to `sftp.root` in `into_server_config()`.
This keeps the common case (one top-level chroot) ergonomic while
letting deployments split SCP and SFTP chroots when they need to.

Acceptance criteria

• `scp -P file user@host:/abs/path/file` succeeds and
  writes to exactly `/abs/path/file` when no chroot is configured.
• `scp -P file user@host:/abs/dir/` writes to
  `/abs/dir/file` when the target is an existing directory.
• `bssh upload local /abs/remote.bin` succeeds without `No such
  file` errors against a server with no chroot.
• When `sftp.root`/`scp.root` is configured, requests outside the
  chroot are rejected and requests inside succeed without path doubling.
• Setting `sftp.root` actually changes the SFTP chroot root.
• `target_is_directory` is honored in `receive_file`.
• 21 integration tests pass, including 6 regression tests for the
  intermediate-directory-symlink security fix.
• Docs describe the new behavior, migration note, and security fix.

Test plan

• `cargo test --lib` — 1204 tests pass.
• `cargo test --test scp_sftp_path_resolution_test` — 21/21 pass.
• `cargo fmt --check` clean.
• `cargo clippy --lib --tests` — no new warnings (two pre-existing
  out-of-scope warnings in vendored `bssh-russh-sftp` and
  `ssh_config/path.rs` unchanged).
• Manual end-to-end validation by reporter (@Yaminyam) against the
  next pre-release build.

Closes #186

[inureyes]

PR security & performance review

Summary: Found and fixed one CRITICAL chroot bypass during review. Path-doubling fix and filename-appending fix verified clean. Performance is fine.

CRITICAL fixed in 9903dd8

Chroot bypass via intermediate-directory symlinks.

Both ScpHandler::resolve_path and SftpHandler::resolve_path_static checked only lexical containment (PathBuf::starts_with(root)) for paths whose final component did not exist (the typical create/mkdir flow). If a symlink existed inside the chroot pointing to a directory outside the chroot — chroot/escape -> /etc, however it got there — a client could target chroot/escape/passwd and have OpenOptions::open(...) follow the symlink to write /etc/passwd. The lexical check saw the path as inside the chroot.

Reproduced with the unmodified PR branch:

let escape_link = chroot.join("escape");
std::os::unix::fs::symlink(&outside_dir, &escape_link).unwrap();
let resolved = handler.resolve_path(&chroot.join("escape").join("newfile.txt"))?;
// Lexically inside the chroot — passed.
OpenOptions::new().write(true).create(true).truncate(true)
    .open(&resolved)?
    .write_all(b"escaped!")?;
// File written to outside_dir/newfile.txt

Fix: when the target path does not exist, walk up to the closest existing ancestor, canonicalize both that ancestor and the chroot root, and verify canonical_ancestor.starts_with(&canonical_root). If the chroot root itself doesn't canonicalize (operator misconfig), fall back to the existing lexical check rather than over-rejecting.

The existing canonicalization on existing paths is preserved unchanged. The fix adds an else if branch for non-existent paths.

Six regression tests added in tests/scp_sftp_path_resolution_test.rs:

• SCP/SFTP file create through parent symlink — blocked
• SFTP mkdir through parent symlink — blocked
• SCP relative client path through parent symlink — blocked
• SCP/SFTP legitimate nested create — still works (sanity)

Verified clean

• Lexical chroot bypass via crafted absolute paths. Path::starts_with is component-aware, so /home/work2 is correctly distinguished from /home/work. Trailing slashes and double-slashes normalize correctly. /../etc/passwd produces /.. which fails the starts_with check. /home/work/../etc/passwd normalizes to /home/etc/passwd and is rejected.
• .. clamp ordering. The existing canonicalize-on-exist branch correctly catches symlink escape via final-component symlinks. The new branch catches it via parent-component symlinks. The relative-path .. clamping in resolve_chroot is component-by-component, with if !resolved.starts_with(root) { resolved = root.to_path_buf(); } as a defense-in-depth backstop.
• No-chroot mode. Absolute paths verbatim, no implicit chroot assumption elsewhere in the code.
• Default behavior change. Documented in CHANGELOG and docs/security.md. Migration note is correct.
• Filename appending decision. The at_top_level && !target_is_directory && !recursive && !target_dir.is_dir() predicate matches OpenSSH and the issue's acceptance criteria.
• Information disclosure. Error messages (Access denied: path outside root, permission_denied) are generic; the operator-controlled chroot config is not leaked.
• Config precedence. scp.root falling back to sftp.root errs on the side of more restriction. Documented in docs/security.md.

Performance pass

No HIGH issues. canonicalize is called once per resolution under chroot mode (one extra realpath(3) walk for non-existent paths). normalize_components allocations are bounded by path length; not in a hot loop. The PathBuf -> Option<PathBuf> refactor adds two .clone() calls per session (handler construction), not per request.

Out of scope (informational)

• TOCTOU between canonicalize and open. Path-based APIs cannot fully prevent symlink swap-in attacks without O_NOFOLLOW + *at syscalls. Pre-existing limitation; PR docs claim symlink-escape protection holds, which is now true for the demonstrated parent-symlink case but a determined attacker with concurrent write access can still race the resolve/open window. Worth a follow-up if hostile-tenant scenarios are in scope.
• Dead code in resolve_chroot_scp. The trailing if !resolved.starts_with(root) after relative-path clamping is unreachable; the per-component clamping ensures the loop invariant. Cosmetic.

[inureyes]

PR Finalization Complete

Tests

• 21 integration tests pass (15 original + 6 regression tests for the chroot-bypass security fix). All 9 acceptance criteria from issue bssh-server: SCP/SFTP path resolution prepends user home to absolute paths and ignores configured sftp.root #186 are covered.
• Full lib test suite: 1204 tests pass (unchanged from before).
• Three pre-existing integration_test.rs failures that require a running localhost SSH server remain out of scope.

Documentation

• CHANGELOG.md: Already contained the complete bug-fix entry, migration note, new scp.root field, and security fix note. No changes needed.
• docs/security.md: Already covered the chroot semantics including the parent-symlink protection. No changes needed.
• docs/architecture/server-configuration.md: Already documented sftp.root and scp.root with new semantics and fallback behavior. No changes needed.
• docs/man/bssh-server.8: Updated — added root field documentation for the sftp and scp sections, added intermediate-directory-symlink protection to SECURITY CONSIDERATIONS, bumped version to v2.1.3.
• README.md: Does not document server configuration or file transfer behavior; no changes needed.
• No Korean docs exist in this repository; no translations to update.

Code quality

• cargo fmt --check clean.
• cargo clippy --lib --tests: only the two pre-existing out-of-scope warnings (bssh-russh-sftp, ssh_config/path.rs). No new warnings.

LOW finding (dead code)

Removed the dead if !resolved.starts_with(root) check in the ParentDir arm of the relative-path chroot resolver in both scp.rs and sftp.rs. The guard is unreachable because resolved starts at root and the preceding if resolved != root guard prevents any pop that could take it above root. Documented the invariant in a comment. Committed as 3eade1b.

All checks passing. Ready for merge.