Skip to content

feat: Implement SFTP server handler with path traversal prevention (#132)#151

Merged
inureyes merged 3 commits intomainfrom
feature/issue-132-implement-sftp-server-handler
Jan 22, 2026
Merged

feat: Implement SFTP server handler with path traversal prevention (#132)#151
inureyes merged 3 commits intomainfrom
feature/issue-132-implement-sftp-server-handler

Conversation

@inureyes
Copy link
Member

Summary

  • Implement SFTP server subsystem using russh-sftp library for file transfer operations
  • Add SftpHandler struct implementing russh_sftp::server::Handler trait with all SFTP operations
  • Implement path traversal prevention to ensure clients cannot access files outside root directory
  • Integrate SFTP handler with SSH handler's subsystem_request for seamless subsystem handling
  • Add comprehensive unit tests for path resolution and security

Changes

New Files

  • src/server/sftp.rs - Complete SFTP handler implementation with:
    • File operations: open, read, write, close
    • Directory operations: opendir, readdir, mkdir, rmdir
    • Attribute operations: stat, lstat, fstat, setstat, fsetstat
    • Path operations: realpath, rename, remove, readlink, symlink
    • Security: resolve_path() method with chroot-like path isolation

Modified Files

  • src/server/handler.rs - Updated subsystem_request to handle SFTP subsystem
  • src/server/mod.rs - Added sftp module export
  • src/server/session.rs - Enhanced ChannelState to store Channel for subsystem use

Security

The SFTP handler implements path traversal prevention by:

  1. All client paths are resolved relative to user's root directory
  2. Parent directory references (..) are clamped to stay within root
  3. Resolved paths are verified to start with root directory before operations

Test plan

  • Path traversal prevention tests pass
  • Handle uniqueness tests pass
  • Error conversion tests pass
  • All 856 existing tests pass
  • Clippy lints pass
  • Code formatted with cargo fmt

Closes #132

Implement the SFTP server subsystem using russh-sftp library for file
transfer operations. The handler provides secure file operations with
chroot-like path resolution that prevents clients from accessing files
outside their designated root directory.
@inureyes inureyes added type:enhancement New feature or request priority:high High priority issue labels Jan 22, 2026
Fix multiple CRITICAL and HIGH security issues in the SFTP server implementation:

CRITICAL fixes:
- Validate symlink targets in symlink() to prevent path traversal via malicious symlinks
- Validate symlink targets in open() and stat() operations before following them
- Prevent symlinks from pointing outside the root directory

HIGH fixes:
- Redact absolute symlink targets in readlink() that point outside root
- Prevent ".." directory entry from leaking parent directory metadata outside root
- At root boundary, use root's own metadata for ".." instead of actual parent

MEDIUM fixes:
- Add maximum handle limit (1000) to prevent resource exhaustion
- Cap read buffer size to 64KB to prevent memory exhaustion

Security improvements:
- Extract resolve_path_static() helper for reuse in symlink validation
- Use symlink_metadata() instead of metadata() where appropriate
- Add comprehensive validation before creating or following symlinks
- Improve logging for security-related operations

All changes maintain backward compatibility and pass existing tests.
- Add tests for symlink handling in build_longname
- Add tests for edge cases: empty paths, special characters, encoded paths
- Add tests for all SftpError helper methods and conversions
- Add tests for static path resolution method
- Add tests for metadata_to_attrs function
- Update ARCHITECTURE.md with SFTP handler documentation
- Update docs/architecture/README.md with SFTP handler reference
- Fix code formatting issues in sftp.rs
@inureyes
Copy link
Member Author

PR Finalization Report

Project Structure Discovered

  • Project Type: Rust
  • Test Framework: cargo test (unit tests in source modules, integration tests in tests/)
  • Documentation System: Plain markdown (ARCHITECTURE.md, docs/architecture/)
  • Multi-language Docs: No
  • Lint Tools: cargo fmt, cargo clippy

Checklist

Tests

  • Analyzed existing test structure (26 SFTP-related tests now, up from 11)
  • Identified missing test coverage for security-critical code
  • Generated 15 new tests:
    • test_build_longname_symlink - Symlink permission formatting
    • test_build_longname_no_permissions - Graceful handling of missing permissions
    • test_resolve_path_empty_string - Empty path handling
    • test_resolve_path_special_characters - Unicode and space handling
    • test_resolve_path_encoded_traversal - Percent-encoded path components
    • test_resolve_path_multiple_slashes - Slash normalization
    • test_resolve_path_dot_only - Single and double dot handling
    • test_resolve_path_alternating_dots - Complex dot patterns
    • test_sftp_error_helpers - All error helper methods
    • test_sftp_error_display - Error Display implementation
    • test_sftp_error_to_status_code - StatusCode conversion
    • test_sftp_error_from_io_eof - EOF error conversion
    • test_sftp_error_from_io_other - Generic IO error conversion
    • test_handler_creation_with_default_root - Default root directory
    • test_resolve_path_static - Static path resolution method
    • test_metadata_to_attrs - Metadata to FileAttributes conversion
  • All tests passing

Documentation

  • ARCHITECTURE.md updated with:
    • Added sftp.rs to server module structure
    • Added SftpHandler component documentation with security features
  • docs/architecture/README.md updated with SFTP Handler reference

Code Quality

  • cargo fmt - Fixed formatting issues
  • cargo clippy -- -D warnings - All warnings resolved

Changes Made

  1. Added 15 comprehensive unit tests for SFTP handler security-critical code
  2. Fixed code formatting issues (multiline function call formatting)
  3. Updated ARCHITECTURE.md with SFTP handler documentation
  4. Updated docs/architecture/README.md with SFTP handler reference

Verification Results

cargo fmt --check: OK
cargo clippy -- -D warnings: OK  
cargo test: 878 tests passed (26 SFTP-specific tests)

Ready for final review.

@inureyes inureyes merged commit 74e6f2e into main Jan 22, 2026
1 of 2 checks passed
@inureyes inureyes deleted the feature/issue-132-implement-sftp-server-handler branch January 22, 2026 09:31
@inureyes inureyes self-assigned this Jan 24, 2026
@inureyes inureyes added the status:done Completed label Jan 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:high High priority issue status:done Completed type:enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement SFTP server handler

1 participant