diff --git a/app/tasks/upload_to_user_integration.py b/app/tasks/upload_to_user_integration.py index e1cae424..1295f6ce 100644 --- a/app/tasks/upload_to_user_integration.py +++ b/app/tasks/upload_to_user_integration.py @@ -501,12 +501,17 @@ def _upload_email(file_path: str, cfg: dict[str, Any], creds: dict[str, Any], ta msg.attach(part) if use_tls: + import ssl + + tls_context = ssl.create_default_context() with smtplib.SMTP(host, port, timeout=30) as smtp: - smtp.starttls() # nosec B608 + smtp.starttls(context=tls_context) if username and password: smtp.login(username, password) smtp.sendmail(msg["From"], [recipient], msg.as_string()) else: + # Plaintext SMTP — only use when explicitly configured and TLS is unavailable. + # Credentials and content will be transmitted without encryption. with smtplib.SMTP(host, port, timeout=30) as smtp: # nosec B608 if username and password: smtp.login(username, password) @@ -518,6 +523,9 @@ def _upload_email(file_path: str, cfg: dict[str, Any], creds: dict[str, Any], ta def _upload_rclone(file_path: str, cfg: dict[str, Any], creds: dict[str, Any], task_id: str) -> dict[str, Any]: """Copy *file_path* to an rclone remote using per-user rclone config.""" + import re + import tempfile + remote = cfg.get("remote") or "" folder = cfg.get("folder") or "" rclone_conf_text = creds.get("rclone_conf") or "" @@ -528,9 +536,17 @@ def _upload_rclone(file_path: str, cfg: dict[str, Any], creds: dict[str, Any], t if not rclone_conf_text: raise ValueError("Rclone integration is missing rclone_conf in credentials") - # Write the user's rclone config to a temp file so we don't touch the system config - import tempfile + # Validate remote and folder to prevent shell metacharacter injection. + # rclone remote names are alphanumeric + hyphens/underscores followed by ':'. + # folder paths must not contain shell-dangerous characters. + _SAFE_REMOTE_RE = re.compile(r"^[A-Za-z0-9_\-]+:(/[A-Za-z0-9_.@\-/ ]*)?$") + _SAFE_FOLDER_RE = re.compile(r"^[A-Za-z0-9_.@\-/ ]*$") + if not _SAFE_REMOTE_RE.match(remote): + raise ValueError(f"Rclone remote contains unsafe characters: {remote!r}") + if folder and not _SAFE_FOLDER_RE.match(folder): + raise ValueError(f"Rclone folder contains unsafe characters: {folder!r}") + # Write the user's rclone config to a temp file so we don't touch the system config with tempfile.NamedTemporaryFile(mode="w", suffix=".conf", delete=False) as tmp_conf: tmp_conf.write(rclone_conf_text) conf_path = tmp_conf.name diff --git a/tests/test_finalize_storage.py b/tests/test_finalize_storage.py index 392e7e13..9b5c594e 100644 --- a/tests/test_finalize_storage.py +++ b/tests/test_finalize_storage.py @@ -19,16 +19,17 @@ def _make_file_record(file_id: int = 123, owner_id=None): return rec +@pytest.fixture(autouse=True) +def _patch_celery_background_tasks(mocker): + """Module-level autouse fixture: prevent lazy-imported Celery tasks from connecting to Redis.""" + mocker.patch("app.tasks.compute_embedding.compute_document_embedding") + mocker.patch("app.tasks.convert_to_pdfa.convert_to_pdfa", create=True) + + @pytest.mark.unit class TestFinalizeDocumentStorage: """Tests for finalize_document_storage Celery task.""" - @pytest.fixture(autouse=True) - def _patch_celery_tasks(self, mocker): - """Prevent all lazy-imported Celery tasks from actually connecting to Redis.""" - mocker.patch("app.tasks.compute_embedding.compute_document_embedding") - mocker.patch("app.tasks.convert_to_pdfa.convert_to_pdfa", create=True) - @patch("app.tasks.finalize_document_storage.notify_file_processed") @patch("app.tasks.finalize_document_storage.send_to_user_destinations") @patch("app.tasks.finalize_document_storage.send_to_all_destinations") @@ -466,12 +467,6 @@ class TestFinalizeDocumentStorage: class TestFinalizeDocumentStorageUserRouting: """Tests for user-specific destination routing in finalize_document_storage.""" - @pytest.fixture(autouse=True) - def _patch_celery_tasks(self, mocker): - """Prevent all lazy-imported Celery tasks from actually connecting to Redis.""" - mocker.patch("app.tasks.compute_embedding.compute_document_embedding") - mocker.patch("app.tasks.convert_to_pdfa.convert_to_pdfa", create=True) - @patch("app.tasks.finalize_document_storage.notify_file_processed") @patch("app.tasks.finalize_document_storage.send_to_user_destinations") @patch("app.tasks.finalize_document_storage.send_to_all_destinations")