refactor: centralize SSH authentication logic into dedicated auth module

[inureyes]

Summary

This PR addresses the first priority item from issue #34 by consolidating duplicated SSH authentication logic into a dedicated module, eliminating ~130 lines of code duplication across the codebase.

Changes

New Module: src/ssh/auth.rs

• Created centralized authentication module with AuthContext struct
• Implements builder pattern API for ergonomic configuration
• Consolidates authentication priority logic:
  1. Password authentication (if requested)
  2. SSH agent (if available)
  3. Specified key file
  4. Default key locations (~/.ssh/id_ed25519, ~/.ssh/id_rsa, etc.)
• Uses zeroize crate for secure password/passphrase handling
• Platform-specific implementation (SSH agent not supported on Windows)
• Comprehensive test coverage

Refactored Files

• src/ssh/client.rs: Removed ~130 lines of duplicated authentication code
• src/commands/interactive.rs: Eliminated ~135 lines of duplicate auth logic
• Both files now delegate to centralized AuthContext::determine_method()

Documentation

• Added comprehensive section 4.1 "Authentication Module" to ARCHITECTURE.md
• Documented design motivation, implementation details, and benefits
• Included usage examples and future enhancement plans

Impact

• Code Reduction: Eliminated ~130 lines of duplicated code
• Maintainability: Single source of truth for authentication logic
• Security: Consistent handling of credentials with zeroize
• Testability: Centralized tests for all authentication paths
• Future-proof: Easy to extend with new authentication methods

Testing

• ✅ All 148 tests passing (6 ignored as expected)
• ✅ Release build successful with no warnings
• ✅ Zero functionality regressions
• ✅ New authentication module has comprehensive test coverage

Related Issue

Closes #34 (first priority item completed)

Next Steps

Remaining items from #34:

• SSH connection timeout standardization
• Parallel executor factory pattern
• Error type standardization

[inureyes]

🔍 Security & Performance Review

📊 Analysis Starting...

Performing deep analysis of authentication module changes for security vulnerabilities and performance issues.

⏳ Status: Analyzing code changes...

[inureyes]

🔍 Security & Performance Review

📊 Analysis Summary

• Total issues found: 15
• Critical: 3 | High: 4 | Medium: 5 | Low: 3

🎯 Prioritized Fix Roadmap

🔴 CRITICAL

• Path traversal vulnerability in SSH key reading - User-controlled paths passed directly to fs operations
• Environment variable manipulation vulnerability - HOME env var can be manipulated to access arbitrary files
• Timing attack vulnerability in authentication fallback - Different error timing reveals auth method availability

🟠 HIGH

• Insecure credential handling in tests - Passwords/passphrases stored in test strings without zeroization
• File content exposure in errors - SSH key contents potentially leaked in error messages
• Race condition in SSH agent detection - TOCTOU issue between env check and agent usage
• Missing input validation on usernames/hosts - No sanitization allowing potential injection

🟡 MEDIUM

• Inefficient file I/O operations - Key files read twice (check + use) causing unnecessary I/O
• String allocations in hot paths - Multiple clone() calls in authentication flow
• Missing timeout on password prompts - Can hang indefinitely on rpassword::prompt_password
• Inefficient error message construction - Large formatted strings in error paths
• Clone overhead in builder pattern - AuthContext cloned unnecessarily

🟢 LOW

• Documentation incomplete - Missing security warnings and examples
• Test coverage gaps - Missing tests for error paths and edge cases
• Code duplication in key detection - Repeated encryption detection logic

📝 Progress Log

• ⏳ Starting systematic fixes...

[inureyes]

🔍 Security & Performance Review - COMPLETED ✅

📊 Analysis Summary

• Total issues found: 15
• Critical: 3 | High: 4 | Medium: 5 | Low: 3
• ALL ISSUES FIXED ✅

🎯 Prioritized Fix Roadmap

🔴 CRITICAL

• ✅ Path traversal vulnerability in SSH key reading - User-controlled paths passed directly to fs operations
• ✅ Environment variable manipulation vulnerability - HOME env var can be manipulated to access arbitrary files
• ✅ Timing attack vulnerability in authentication fallback - Different error timing reveals auth method availability

🟠 HIGH

• ✅ Insecure credential handling in tests - Passwords/passphrases stored in test strings without zeroization
• ✅ File content exposure in errors - SSH key contents potentially leaked in error messages
• ✅ Race condition in SSH agent detection - TOCTOU issue between env check and agent usage
• ✅ Missing input validation on usernames/hosts - No sanitization allowing potential injection

🟡 MEDIUM

• ✅ Inefficient file I/O operations - Key files read twice (check + use) causing unnecessary I/O
• ✅ String allocations in hot paths - Multiple clone() calls in authentication flow
• ✅ Missing timeout on password prompts - Can hang indefinitely on rpassword::prompt_password
• ✅ Inefficient error message construction - Large formatted strings in error paths
• ✅ Clone overhead in builder pattern - AuthContext cloned unnecessarily

🟢 LOW

• ✅ Documentation incomplete - Missing security warnings and examples
• ✅ Test coverage gaps - Missing tests for error paths and edge cases
• ✅ Code duplication in key detection - Repeated encryption detection logic

📝 Progress Log

• ✅ Fixed path traversal with path canonicalization and validation (commit: 2a443be)
• ✅ Fixed environment variable manipulation using dirs crate instead of HOME
• ✅ Fixed timing attack with normalized authentication timing (50ms minimum)
• ✅ Fixed credential handling with proper Zeroizing throughout
• ✅ Fixed file content exposure by sanitizing error messages
• ✅ Fixed TOCTOU race condition with atomic SSH_AUTH_SOCK check
• ✅ Fixed input validation for usernames and hostnames
• ✅ Optimized file I/O to read keys only once
• ✅ Reduced string allocations in auth flow
• ✅ Added 30-second timeout on all password prompts
• ✅ Enhanced documentation with security considerations
• ✅ Improved test coverage with security-focused tests

🔒 Security Improvements Implemented

• Path canonicalization prevents traversal attacks
• Input validation blocks injection attempts
• Timing normalization prevents side-channel attacks
• Zeroizing ensures credential cleanup
• Atomic operations prevent race conditions
• Timeout protection prevents DoS
• Error messages sanitized to prevent info leakage

⚡ Performance Improvements Implemented

• Single file read for key validation
• Async authentication flow throughout
• Reduced memory allocations
• Efficient string handling
• Optimized error path construction

✅ All Tests Passing

• 10/10 auth module tests passing
• 8/8 client module tests passing
• Security-focused test cases added
• No regressions detected