From b42c6f5f648a9d4c0b337f2bd9244f7e27283e3d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 13 Feb 2026 16:12:39 +0000 Subject: [PATCH] fix: resolve all Ruff linting errors - Fix PLW2901: Use different variable name for stripped lines in loop - Fix E721: Use 'is' instead of '==' for type comparisons - Add noqa comments for intentional security warnings (S321, S507, S110, S603) Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> --- app/api/dropbox.py | 9 ++++----- app/api/google_drive.py | 9 ++++----- app/api/onedrive.py | 9 ++++----- app/tasks/upload_to_ftp.py | 6 +++--- app/tasks/upload_to_sftp.py | 6 +++--- app/tasks/upload_with_rclone.py | 8 ++++---- app/utils/config_loader.py | 8 ++++---- app/views/status.py | 4 ++-- tests/test_check_credentials.py | 5 +---- tests/test_e2e_full_stack.py | 2 +- tests/test_security_headers.py | 22 ++++++++++++++-------- tests/test_upload_email.py | 3 --- tests/test_upload_ftp_additional.py | 8 ++++---- tests/test_upload_google_drive.py | 2 +- tests/test_views_status.py | 3 --- 15 files changed, 49 insertions(+), 55 deletions(-) diff --git a/app/api/dropbox.py b/app/api/dropbox.py index fde72f16..ea7600aa 100644 --- a/app/api/dropbox.py +++ b/app/api/dropbox.py @@ -224,18 +224,17 @@ async def save_dropbox_settings( updated = set() new_env_lines = [] for line in env_lines: - line = line.rstrip() + stripped_line = line.rstrip() is_updated = False for key, value in dropbox_settings.items(): - if line.startswith(f"{key}=") or line.startswith(f"# {key}="): - if line.startswith("# "): # Uncomment if commented out - line = line[2:] + if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="): + # Uncomment if commented out - check the original stripped line new_env_lines.append(f"{key}={value}") updated.add(key) is_updated = True break if not is_updated: - new_env_lines.append(line) + new_env_lines.append(stripped_line) # Add any settings that weren't updated (they weren't in the file) for key, value in dropbox_settings.items(): diff --git a/app/api/google_drive.py b/app/api/google_drive.py index e1942df4..b626a470 100644 --- a/app/api/google_drive.py +++ b/app/api/google_drive.py @@ -373,18 +373,17 @@ async def save_dropbox_settings( updated = set() new_env_lines = [] for line in env_lines: - line = line.rstrip() + stripped_line = line.rstrip() is_updated = False for key, value in drive_settings.items(): - if line.startswith(f"{key}=") or line.startswith(f"# {key}="): - if line.startswith("# "): # Uncomment if commented out - line = line[2:] + if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="): + # Uncomment if commented out - check the original stripped line new_env_lines.append(f"{key}={value}") updated.add(key) is_updated = True break if not is_updated: - new_env_lines.append(line) + new_env_lines.append(stripped_line) # Add any settings that weren't updated (they weren't in the file) for key, value in drive_settings.items(): diff --git a/app/api/onedrive.py b/app/api/onedrive.py index 1b226768..894a1a44 100644 --- a/app/api/onedrive.py +++ b/app/api/onedrive.py @@ -242,18 +242,17 @@ async def save_onedrive_settings( updated = set() new_env_lines = [] for line in env_lines: - line = line.rstrip() + stripped_line = line.rstrip() is_updated = False for key, value in onedrive_settings.items(): - if line.startswith(f"{key}=") or line.startswith(f"# {key}="): - if line.startswith("# "): # Uncomment if commented out - line = line[2:] + if stripped_line.startswith(f"{key}=") or stripped_line.startswith(f"# {key}="): + # Uncomment if commented out - check the original stripped line new_env_lines.append(f"{key}={value}") updated.add(key) is_updated = True break if not is_updated: - new_env_lines.append(line) + new_env_lines.append(stripped_line) # Add any settings that weren't updated (they weren't in the file) for key, value in onedrive_settings.items(): diff --git a/app/tasks/upload_to_ftp.py b/app/tasks/upload_to_ftp.py index 11227068..e6207e34 100644 --- a/app/tasks/upload_to_ftp.py +++ b/app/tasks/upload_to_ftp.py @@ -57,7 +57,7 @@ def upload_to_ftp(self, file_path: str, file_id: int = None): if use_tls: try: logger.info(f"Attempting FTPS connection to {settings.ftp_host}") - ftp = ftplib.FTP_TLS() + ftp = ftplib.FTP_TLS() # noqa: S321 ftp.connect(host=settings.ftp_host, port=settings.ftp_port or 21) # Login with credentials @@ -74,7 +74,7 @@ def upload_to_ftp(self, file_path: str, file_id: int = None): else: logger.warning(f"FTPS connection failed, falling back to regular FTP: {str(e)}") # Fall back to regular FTP - only if explicitly allowed by configuration - ftp = ftplib.FTP() # nosec B321 - Fallback to FTP intentional when configured + ftp = ftplib.FTP() # nosec B321 - Fallback to FTP intentional when configured # noqa: S321 ftp.connect(host=settings.ftp_host, port=settings.ftp_port or 21) # Login with credentials @@ -88,7 +88,7 @@ def upload_to_ftp(self, file_path: str, file_id: int = None): # Directly use regular FTP if TLS is explicitly disabled logger.warning("Using plaintext FTP - connection is NOT encrypted!") - ftp = ftplib.FTP() # nosec B321 - Plaintext FTP intentional when explicitly configured + ftp = ftplib.FTP() # nosec B321 - Plaintext FTP intentional when explicitly configured # noqa: S321 ftp.connect(host=settings.ftp_host, port=settings.ftp_port or 21) # Login with credentials diff --git a/app/tasks/upload_to_sftp.py b/app/tasks/upload_to_sftp.py index 8e0676dd..98da9261 100644 --- a/app/tasks/upload_to_sftp.py +++ b/app/tasks/upload_to_sftp.py @@ -56,7 +56,7 @@ def upload_to_sftp(self, file_path: str, file_id: int = None): "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 + ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) # nosec B507 - Configurable, warns user # noqa: S507 else: # Use system known_hosts for host key verification (more secure) ssh.load_system_host_keys() @@ -149,8 +149,8 @@ def upload_to_sftp(self, file_path: str, file_id: int = None): if "sftp" in locals(): sftp.close() ssh.close() - except Exception: - pass + except Exception: # noqa: S110 + pass # Ignore errors during cleanup error_msg = f"Failed to upload {filename} to SFTP server: {str(e)}" logger.error(f"[{task_id}] {error_msg}") diff --git a/app/tasks/upload_with_rclone.py b/app/tasks/upload_with_rclone.py index 59b83f71..1d02d374 100644 --- a/app/tasks/upload_with_rclone.py +++ b/app/tasks/upload_with_rclone.py @@ -57,7 +57,7 @@ def upload_with_rclone(self, file_path: str, destination: str): # Ensure the remote path exists (create folders if needed) mkdir_cmd = ["rclone", "mkdir", "--config", rclone_config_path, destination] - subprocess.run(mkdir_cmd, check=True, capture_output=True) + subprocess.run(mkdir_cmd, check=True, capture_output=True) # noqa: S603 # Construct the upload command upload_cmd = ["rclone", "copy", "--config", rclone_config_path, file_path, destination, "--progress"] @@ -65,14 +65,14 @@ def upload_with_rclone(self, file_path: str, destination: str): log_task_progress(task_id, "rclone_upload", "in_progress", f"Executing rclone copy to {destination}") # Execute the upload command - result = subprocess.run(upload_cmd, check=True, capture_output=True, text=True) + result = subprocess.run(upload_cmd, check=True, capture_output=True, text=True) # noqa: S603 # Check if upload was successful if result.returncode == 0: # Try to get a public link if possible try: link_cmd = ["rclone", "link", "--config", rclone_config_path, f"{destination}/{filename}"] - link_result = subprocess.run(link_cmd, capture_output=True, text=True, check=False) + link_result = subprocess.run(link_cmd, capture_output=True, text=True, check=False) # noqa: S603 public_url = link_result.stdout.strip() if link_result.returncode == 0 else None except (subprocess.SubprocessError, OSError) as e: logger.warning(f"[{task_id}] Failed to get public link for {filename}: {str(e)}") @@ -135,7 +135,7 @@ def send_to_all_rclone_destinations(self, file_path: str): # Get list of configured destinations from rclone try: remotes_cmd = ["rclone", "listremotes", "--config", rclone_config_path] - result = subprocess.run(remotes_cmd, check=True, capture_output=True, text=True) + result = subprocess.run(remotes_cmd, check=True, capture_output=True, text=True) # noqa: S603 if result.returncode == 0: # Process the list of remotes diff --git a/app/utils/config_loader.py b/app/utils/config_loader.py index 8d251712..2efe846a 100644 --- a/app/utils/config_loader.py +++ b/app/utils/config_loader.py @@ -87,21 +87,21 @@ def convert_setting_value(value: Optional[str], field_type: Any) -> Any: field_type = next((arg for arg in args if arg is not type(None)), str) # Convert based on type - if field_type == bool: + if field_type is bool: return value.lower() in ("true", "1", "yes", "y", "t") - elif field_type == int: + elif field_type is int: try: return int(value) except ValueError: logger.warning(f"Failed to convert '{value}' to int, returning 0") return 0 - elif field_type == float: + elif field_type is float: try: return float(value) except ValueError: logger.warning(f"Failed to convert '{value}' to float, returning 0.0") return 0.0 - elif field_type == list or getattr(field_type, "__origin__", None) == list: + elif field_type is list or getattr(field_type, "__origin__", None) is list: # Handle list types - assume comma-separated values if isinstance(value, str): return [item.strip() for item in value.split(",") if item.strip()] diff --git a/app/views/status.py b/app/views/status.py index da883068..6c5fbe52 100644 --- a/app/views/status.py +++ b/app/views/status.py @@ -57,8 +57,8 @@ async def status_dashboard(request: Request): # Try to get runtime information try: container_info["runtime_info"] = settings.runtime_info - except Exception: - pass + except Exception: # noqa: S110 + pass # Ignore if runtime_info not available else: container_info["is_docker"] = False diff --git a/tests/test_check_credentials.py b/tests/test_check_credentials.py index 31a684df..4aa93c3c 100644 --- a/tests/test_check_credentials.py +++ b/tests/test_check_credentials.py @@ -2,7 +2,7 @@ import json import os -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patch import pytest @@ -12,9 +12,6 @@ from app.tasks.check_credentials import ( get_failure_state, save_failure_state, sync_test_azure_connection, - sync_test_dropbox_token, - sync_test_google_drive_token, - sync_test_onedrive_token, sync_test_openai_connection, unwrap_decorated_function, ) diff --git a/tests/test_e2e_full_stack.py b/tests/test_e2e_full_stack.py index 1efa1279..1573c2c3 100644 --- a/tests/test_e2e_full_stack.py +++ b/tests/test_e2e_full_stack.py @@ -449,7 +449,7 @@ class TestFullInfrastructure: # Create SFTP client ssh = paramiko.SSHClient() - ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) + ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) # noqa: S507 # Connect to SFTP server ssh.connect( diff --git a/tests/test_security_headers.py b/tests/test_security_headers.py index 203ce29d..4a22f306 100644 --- a/tests/test_security_headers.py +++ b/tests/test_security_headers.py @@ -207,9 +207,10 @@ class TestSecurityHeadersMiddleware: @pytest.mark.asyncio async def test_dispatch_adds_headers_when_enabled(self): """Test dispatch adds security headers when enabled.""" + from fastapi import Response + from app.config import settings from app.middleware.security_headers import SecurityHeadersMiddleware - from fastapi import Response if not settings.security_headers_enabled: pytest.skip("Security headers disabled in configuration") @@ -231,10 +232,10 @@ class TestSecurityHeadersMiddleware: @pytest.mark.asyncio async def test_dispatch_skips_headers_when_disabled(self): """Test dispatch skips headers when disabled.""" - from app.config import settings - from app.middleware.security_headers import SecurityHeadersMiddleware from fastapi import Response + from app.middleware.security_headers import SecurityHeadersMiddleware + # Create a config copy with headers disabled mock_config = Mock() mock_config.security_headers_enabled = False @@ -254,9 +255,10 @@ class TestSecurityHeadersMiddleware: def test_add_security_headers_hsts(self): """Test _add_security_headers adds HSTS header.""" - from app.middleware.security_headers import SecurityHeadersMiddleware from fastapi import Response + from app.middleware.security_headers import SecurityHeadersMiddleware + mock_config = Mock() mock_config.security_headers_enabled = True mock_config.security_header_hsts_enabled = True @@ -275,9 +277,10 @@ class TestSecurityHeadersMiddleware: def test_add_security_headers_csp(self): """Test _add_security_headers adds CSP header.""" - from app.middleware.security_headers import SecurityHeadersMiddleware from fastapi import Response + from app.middleware.security_headers import SecurityHeadersMiddleware + mock_config = Mock() mock_config.security_headers_enabled = True mock_config.security_header_hsts_enabled = False @@ -296,9 +299,10 @@ class TestSecurityHeadersMiddleware: def test_add_security_headers_x_frame_options(self): """Test _add_security_headers adds X-Frame-Options header.""" - from app.middleware.security_headers import SecurityHeadersMiddleware from fastapi import Response + from app.middleware.security_headers import SecurityHeadersMiddleware + mock_config = Mock() mock_config.security_headers_enabled = True mock_config.security_header_hsts_enabled = False @@ -317,9 +321,10 @@ class TestSecurityHeadersMiddleware: def test_add_security_headers_x_content_type_options(self): """Test _add_security_headers adds X-Content-Type-Options header.""" - from app.middleware.security_headers import SecurityHeadersMiddleware from fastapi import Response + from app.middleware.security_headers import SecurityHeadersMiddleware + mock_config = Mock() mock_config.security_headers_enabled = True mock_config.security_header_hsts_enabled = False @@ -337,9 +342,10 @@ class TestSecurityHeadersMiddleware: def test_add_all_security_headers(self): """Test _add_security_headers adds all headers when all enabled.""" - from app.middleware.security_headers import SecurityHeadersMiddleware from fastapi import Response + from app.middleware.security_headers import SecurityHeadersMiddleware + mock_config = Mock() mock_config.security_headers_enabled = True mock_config.security_header_hsts_enabled = True diff --git a/tests/test_upload_email.py b/tests/test_upload_email.py index cf9e319c..e8d6adf2 100644 --- a/tests/test_upload_email.py +++ b/tests/test_upload_email.py @@ -1,11 +1,8 @@ """Tests for app/tasks/upload_to_email.py module.""" import json -import os -import smtplib import socket from email.mime.multipart import MIMEMultipart -from pathlib import Path from unittest.mock import MagicMock, Mock, mock_open, patch import pytest diff --git a/tests/test_upload_ftp_additional.py b/tests/test_upload_ftp_additional.py index 03b5b3ee..86d1225c 100644 --- a/tests/test_upload_ftp_additional.py +++ b/tests/test_upload_ftp_additional.py @@ -1,7 +1,7 @@ """Additional tests for upload_to_ftp task.""" import ftplib -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patch import pytest @@ -152,7 +152,7 @@ class TestUploadToFtp: mock_settings.ftp_use_tls = True mock_ftp = Mock() - mock_ftp.cwd.side_effect = [ftplib.error_perm("No such directory"), None] + mock_ftp.cwd.side_effect = [ftplib.error_perm("No such directory"), None] # noqa: S321 mock_ftp_tls.return_value = mock_ftp mock_self = Mock() @@ -248,8 +248,8 @@ class TestUploadToFtp: mock_settings.ftp_use_tls = True mock_ftp = Mock() - mock_ftp.cwd.side_effect = ftplib.error_perm("Permission denied") - mock_ftp.mkd.side_effect = ftplib.error_perm("Cannot create directory") + mock_ftp.cwd.side_effect = ftplib.error_perm("Permission denied") # noqa: S321 + mock_ftp.mkd.side_effect = ftplib.error_perm("Cannot create directory") # noqa: S321 mock_ftp_tls.return_value = mock_ftp mock_self = Mock() diff --git a/tests/test_upload_google_drive.py b/tests/test_upload_google_drive.py index ef8d21b1..6b1882b9 100644 --- a/tests/test_upload_google_drive.py +++ b/tests/test_upload_google_drive.py @@ -1,7 +1,7 @@ """Tests for app/tasks/upload_to_google_drive.py module.""" import json -from unittest.mock import MagicMock, Mock, mock_open, patch +from unittest.mock import Mock, patch import pytest from google.auth.exceptions import RefreshError diff --git a/tests/test_views_status.py b/tests/test_views_status.py index decc1c25..956e7e84 100644 --- a/tests/test_views_status.py +++ b/tests/test_views_status.py @@ -240,7 +240,6 @@ class TestContainerInfoDetection: @patch("builtins.open", new_callable=mock_open, read_data="12:docker:/abc123456789") def test_extracts_container_id(self, mock_file, mock_exists): """Test extracts container ID from cgroup.""" - from app.views.status import status_dashboard mock_exists.return_value = True @@ -250,7 +249,6 @@ class TestContainerInfoDetection: @patch("app.views.status.os.path.exists") def test_handles_missing_cgroup_file(self, mock_exists): """Test handles missing cgroup file gracefully.""" - from app.views.status import status_dashboard mock_exists.side_effect = [True, False] # Docker env exists, but cgroup doesn't @@ -259,7 +257,6 @@ class TestContainerInfoDetection: @patch("app.views.status.settings") def test_includes_runtime_info_when_available(self, mock_settings): """Test includes runtime info when available.""" - from app.views.status import status_dashboard mock_settings.runtime_info = "Python 3.11.5 on Linux"