diff --git a/app/api/dropbox.py b/app/api/dropbox.py index 86f72a25..e2d33448 100644 --- a/app/api/dropbox.py +++ b/app/api/dropbox.py @@ -8,6 +8,7 @@ from typing import Annotated, Optional from urllib.parse import quote import httpx +import requests from fastapi import APIRouter, Depends, Form, HTTPException, Request, status from sqlalchemy.orm import Session diff --git a/app/api/onedrive.py b/app/api/onedrive.py index 9e19c303..cf39f43b 100644 --- a/app/api/onedrive.py +++ b/app/api/onedrive.py @@ -7,6 +7,7 @@ from datetime import datetime, timedelta from typing import Annotated, Optional import httpx +import requests from fastapi import APIRouter, Depends, Form, HTTPException, Request, status from sqlalchemy.orm import Session diff --git a/app/tasks/convert_to_pdfa.py b/app/tasks/convert_to_pdfa.py index b94ae74a..6def4d24 100644 --- a/app/tasks/convert_to_pdfa.py +++ b/app/tasks/convert_to_pdfa.py @@ -78,6 +78,7 @@ def _convert_pdf_to_pdfa(input_path: str, output_path: str, pdfa_format: str = " output_type, "--quiet", "--invalidate-digital-signatures", + "--", input_path, output_path, ] diff --git a/app/tasks/upload_to_user_integration.py b/app/tasks/upload_to_user_integration.py index 1b23d321..9654fb5c 100644 --- a/app/tasks/upload_to_user_integration.py +++ b/app/tasks/upload_to_user_integration.py @@ -555,8 +555,9 @@ def _upload_rclone(file_path: str, cfg: dict[str, Any], creds: dict[str, Any], t dest = dest.replace("//", "/") try: + # SECURITY: Separate options from positional arguments using -- to prevent command injection result = subprocess.run( # nosec B603 # noqa: S603 S607 - ["rclone", "copyto", f"--config={conf_path}", file_path, dest], # noqa: S603 S607 + ["rclone", "copyto", f"--config={conf_path}", "--", file_path, dest], # noqa: S603 S607 capture_output=True, text=True, timeout=300, diff --git a/tests/test_convert_to_pdfa.py b/tests/test_convert_to_pdfa.py index 12ee6243..734ca988 100644 --- a/tests/test_convert_to_pdfa.py +++ b/tests/test_convert_to_pdfa.py @@ -38,6 +38,11 @@ class TestConvertPdfToPdfa: assert "pdfa-2" in cmd assert "--quiet" in cmd assert "--invalidate-digital-signatures" in cmd + # SECURITY: Verify `--` end-of-options separator is present and precedes + # the file paths to prevent option/argument injection. + assert "--" in cmd + assert cmd.index("--") < cmd.index("/input.pdf") + assert cmd.index("--") < cmd.index("/output.pdf") assert "/input.pdf" in cmd assert "/output.pdf" in cmd diff --git a/tests/test_upload_handlers.py b/tests/test_upload_handlers.py index e18e26d5..192cdf98 100644 --- a/tests/test_upload_handlers.py +++ b/tests/test_upload_handlers.py @@ -769,6 +769,11 @@ class TestUploadRclone: cmd = mock_run.call_args[0][0] assert cmd[0] == "rclone" assert cmd[1] == "copyto" + # SECURITY: Verify `--` end-of-options separator is present and precedes + # the file path and destination to prevent option/argument injection. + assert "--" in cmd + fp_index = next(i for i, v in enumerate(cmd) if v == fp) + assert cmd.index("--") < fp_index def test_raises_on_rclone_nonzero_exit(self, tmp_path): fp = str(tmp_path / "doc.pdf")