From 9a89f70cf4f18664867efc6f7036ff228af873bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 15:39:23 +0000 Subject: [PATCH] fix: change SFTP host key verification default to secure (False) Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- .env.demo | 5 +++-- SECURITY_AUDIT.md | 10 +++++----- app/config.py | 6 +++--- app/tasks/upload_to_sftp.py | 9 +++++---- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/.env.demo b/.env.demo index fb4752f2..c5688061 100644 --- a/.env.demo +++ b/.env.demo @@ -174,7 +174,8 @@ FTP_USE_TLS=True FTP_ALLOW_PLAINTEXT=True # SFTP -# Security Note: Set SFTP_DISABLE_HOST_KEY_VERIFICATION=false in production +# Security Note: Host key verification is enabled by default (False) +# Only set to True in development/testing environments if needed # When false, configure SSH known_hosts for proper host key verification SFTP_HOST=sftp.example.com SFTP_PORT=22 @@ -183,7 +184,7 @@ SFTP_PASSWORD=your_secure_sftp_password # SFTP_PRIVATE_KEY=/path/to/private_key.pem # SFTP_PRIVATE_KEY_PASSPHRASE=optional_passphrase SFTP_FOLDER=/Documents/Uploads -SFTP_DISABLE_HOST_KEY_VERIFICATION=True # Set to False in production for security +SFTP_DISABLE_HOST_KEY_VERIFICATION=False # Default is False (secure); set to True only for testing # **HTTP Request Settings** # Timeout for HTTP requests - set higher to handle large PDF files (up to 1GB) diff --git a/SECURITY_AUDIT.md b/SECURITY_AUDIT.md index 08097901..7b9eac99 100644 --- a/SECURITY_AUDIT.md +++ b/SECURITY_AUDIT.md @@ -55,13 +55,13 @@ This document tracks security vulnerabilities found in DocuElevate and their rem **Issue:** Using `paramiko.AutoAddPolicy()` automatically trusts unknown SSH host keys, making connections vulnerable to MITM attacks. **Remediation:** -- Added configuration option `sftp_disable_host_key_verification` (default: True for backward compatibility) -- When disabled (production recommended), uses `paramiko.RejectPolicy()` with system known_hosts -- Added prominent security warnings when host key verification is disabled -- Added `# nosec B507` annotation with justification +- Added configuration option `sftp_disable_host_key_verification` (default: False for security) +- When enabled (False), uses `paramiko.RejectPolicy()` with system known_hosts for secure verification +- When disabled (True, for testing only), uses `AutoAddPolicy()` with security warnings +- Added `# nosec B507` annotation with justification for the test/dev use case - Updated docstrings with security guidance -**Production Recommendation:** Set `SFTP_DISABLE_HOST_KEY_VERIFICATION=false` and configure SSH known_hosts file. +**Security Note:** The default value is now `False` (secure). For development/testing environments where host keys cannot be pre-configured, set `SFTP_DISABLE_HOST_KEY_VERIFICATION=True` (not recommended for production). #### 4. B113: Missing Timeout on HTTP Requests (MEDIUM SEVERITY) ✅ FIXED **Occurrences:** 15 diff --git a/app/config.py b/app/config.py index 60b1a950..fd6beeac 100644 --- a/app/config.py +++ b/app/config.py @@ -107,9 +107,9 @@ class Settings(BaseSettings): sftp_folder: Optional[str] = None sftp_private_key: Optional[str] = None sftp_private_key_passphrase: Optional[str] = None - # Security: Disable host key verification only in development/testing environments - # In production, set to True and configure known_hosts file - sftp_disable_host_key_verification: bool = True # Default allows connection without known_hosts + # Security: Host key verification is enabled by default for security + # In development/testing, set to True to disable verification (not recommended) + sftp_disable_host_key_verification: bool = False # Default enforces host key verification # Email settings email_host: Optional[str] = None diff --git a/app/tasks/upload_to_sftp.py b/app/tasks/upload_to_sftp.py index 97578e91..8e0676dd 100644 --- a/app/tasks/upload_to_sftp.py +++ b/app/tasks/upload_to_sftp.py @@ -48,12 +48,13 @@ def upload_to_sftp(self, file_path: str, file_id: int = None): ssh = paramiko.SSHClient() # Security: Host key verification - # WARNING: AutoAddPolicy automatically trusts unknown host keys (vulnerable to MITM attacks) - # For production, use RejectPolicy and configure known_hosts, or WarningPolicy at minimum - if getattr(settings, "sftp_disable_host_key_verification", True): + # By default (sftp_disable_host_key_verification=False), we use RejectPolicy for security + # Setting sftp_disable_host_key_verification=True disables verification (for testing only) + if getattr(settings, "sftp_disable_host_key_verification", False): logger.warning( "SFTP host key verification is DISABLED - connections are vulnerable to MITM attacks. " - "For production, set SFTP_DISABLE_HOST_KEY_VERIFICATION=false and configure known_hosts." + "This should only be used in development/testing. For production, remove " + "SFTP_DISABLE_HOST_KEY_VERIFICATION or set it to False and configure known_hosts." ) ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) # nosec B507 - Configurable, warns user else: