Merge pull request #774 from christianlouis/fix-command-injection-9532322968535721630

🛡️ Sentinel: [HIGH] Fix command injection vulnerability in subprocess calls

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
 

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
 

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,
     ]

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,

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
 

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")