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>
This commit is contained in:
+4
-5
@@ -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():
|
||||
|
||||
@@ -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():
|
||||
|
||||
+4
-5
@@ -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():
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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}")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()]
|
||||
|
||||
+2
-2
@@ -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
|
||||
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user