🛡️ Sentinel: [CRITICAL] Fix LaTeX Injection RCE and DoS in PDF Compilation

.jules/sentinel.md

@@ -7,3 +7,8 @@
 **Vulnerability:** The `CoverLetterGenerator` used a standard Jinja2 environment (intended for HTML/XML or plain text) to render LaTeX templates. This allowed malicious user input (or AI hallucinations) containing LaTeX control characters (e.g., `\input{...}`) to be injected directly into the LaTeX source, leading to potential Local File Inclusion (LFI) or other exploits.
 **Learning:** Jinja2's default `autoescape` is context-aware based on file extensions, but usually only for HTML/XML. It does NOT automatically escape LaTeX special characters. Relying on manual filters (like `| latex_escape`) in templates is error-prone and brittle, as developers might forget to apply them to every variable.
 **Prevention:** Always use a dedicated Jinja2 environment for LaTeX generation that enforces auto-escaping via a `finalize` hook (e.g., `tex_env.finalize = latex_escape`). This ensures *all* variable output is sanitized by default, providing defense-in-depth even if the template author forgets explicit filters.
+
+## 2025-02-21 - [CRITICAL] LaTeX Injection RCE and DoS in PDF Compilation
+**Vulnerability:** LaTeX compilers (pdflatex and pandoc fallback) were executed without `-no-shell-escape`, allowing LaTeX code to execute arbitrary shell commands if maliciously injected. Additionally, PDF compilation lacked timeouts, leading to potential Denial of Service (DoS) if compilation hangs.
+**Learning:** Command execution involving complex parsers like LaTeX must always restrict dangerous features (like shell escapes) and enforce timeouts, even if the primary input is not directly user-provided, as upstream generators or AI could inject payloads.
+**Prevention:** Always append `-no-shell-escape` for pdflatex and `--pdf-engine-opt=-no-shell-escape` for pandoc. Enforce a timeout in `process.communicate(timeout=30)` and safely clean up processes using `process.kill()` on `TimeoutExpired`.

cli/generators/cover_letter_generator.py

@@ -771,12 +771,18 @@ def _compile_pdf(self, output_path: Path, tex_content: str) -> bool:
         try:
             # Use Popen with explicit cleanup to avoid double-free issues
             process = subprocess.Popen(
-                ["pdflatex", "-interaction=nonstopmode", tex_path.name],
+                ["pdflatex", "-interaction=nonstopmode", "-no-shell-escape", tex_path.name],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
                 cwd=tex_path.parent,
             )
-            stdout, stderr = process.communicate()
+            try:
+                stdout, stderr = process.communicate(timeout=30)
+            except subprocess.TimeoutExpired:
+                process.kill()
+                stdout, stderr = process.communicate()
+                return False
+
             if process.returncode == 0 or output_path.exists():
                 pdf_created = True
         except (subprocess.CalledProcessError, FileNotFoundError):
@@ -787,11 +793,24 @@ def _compile_pdf(self, output_path: Path, tex_content: str) -> bool:
                 # Fallback to pandoc
                 try:
                     process = subprocess.Popen(
-                        ["pandoc", str(tex_path), "-o", str(output_path), "--pdf-engine=xelatex"],
+                        [
+                            "pandoc",
+                            str(tex_path),
+                            "-o",
+                            str(output_path),
+                            "--pdf-engine=xelatex",
+                            "--pdf-engine-opt=-no-shell-escape",
+                        ],
                         stdout=subprocess.PIPE,
                         stderr=subprocess.PIPE,
                     )
-                    stdout, stderr = process.communicate()
+                    try:
+                        stdout, stderr = process.communicate(timeout=30)
+                    except subprocess.TimeoutExpired:
+                        process.kill()
+                        stdout, stderr = process.communicate()
+                        return False
+
                     if process.returncode == 0 or output_path.exists():
                         pdf_created = True
                 except (subprocess.CalledProcessError, FileNotFoundError):

cli/pdf/converter.py

@@ -86,12 +86,17 @@ def _compile_pdflatex(
         """
         try:
             process = subprocess.Popen(
-                ["pdflatex", "-interaction=nonstopmode", tex_path.name],
+                ["pdflatex", "-interaction=nonstopmode", "-no-shell-escape", tex_path.name],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
                 cwd=working_dir,
             )
-            stdout, stderr = process.communicate()
+            try:
+                stdout, stderr = process.communicate(timeout=30)
+            except subprocess.TimeoutExpired:
+                process.kill()
+                stdout, stderr = process.communicate()
+                return False
 
             if process.returncode == 0 or output_path.exists():
                 return True
@@ -121,12 +126,24 @@ def _compile_pandoc(
         """
         try:
             process = subprocess.Popen(
-                ["pandoc", str(tex_path), "-o", str(output_path), "--pdf-engine=xelatex"],
+                [
+                    "pandoc",
+                    str(tex_path),
+                    "-o",
+                    str(output_path),
+                    "--pdf-engine=xelatex",
+                    "--pdf-engine-opt=-no-shell-escape",
+                ],
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
                 cwd=working_dir,
             )
-            stdout, stderr = process.communicate()
+            try:
+                stdout, stderr = process.communicate(timeout=30)
+            except subprocess.TimeoutExpired:
+                process.kill()
+                stdout, stderr = process.communicate()
+                return False
 
             if process.returncode == 0 or output_path.exists():
                 return True

tests/test_pdf_security.py

@@ -1,9 +1,12 @@
+import os
 import subprocess
 import unittest
 from pathlib import Path
 from unittest.mock import MagicMock, patch
 
+from cli.generators.cover_letter_generator import CoverLetterGenerator
 from cli.generators.template import TemplateGenerator
+from cli.pdf.converter import PDFConverter
 
 
 class TestPDFSecurity(unittest.TestCase):
@@ -55,6 +58,139 @@ def test_pdflatex_arguments(self, mock_popen):
         self.assertIn("-interaction=nonstopmode", command)
         self.assertIn("pdflatex", command)
 
+    @patch("cli.pdf.converter.subprocess.Popen")
+    def test_converter_pdflatex_timeout(self, mock_popen):
+        # Setup mock
+        process_mock = MagicMock()
+        process_mock.communicate.side_effect = [
+            subprocess.TimeoutExpired(cmd="pdflatex", timeout=30),
+            (b"", b""),
+        ]
+        mock_popen.return_value = process_mock
+
+        converter = PDFConverter()
+
+        # We need to mock output_path.exists to return False to check fallback/failure
+        with patch.object(Path, "exists", return_value=False):
+            result = converter._compile_pdflatex(Path("output.tex"), Path("output.pdf"), Path("."))
+
+        self.assertFalse(result)
+        process_mock.kill.assert_called_once()
+        process_mock.communicate.assert_any_call(timeout=30)
+
+    @patch("cli.pdf.converter.subprocess.Popen")
+    def test_converter_pdflatex_arguments(self, mock_popen):
+        process_mock = MagicMock()
+        process_mock.communicate.return_value = (b"", b"")
+        process_mock.returncode = 0
+        mock_popen.return_value = process_mock
+
+        converter = PDFConverter()
+
+        with patch.object(Path, "exists", return_value=True):
+            converter._compile_pdflatex(Path("output.tex"), Path("output.pdf"), Path("."))
+
+        args, _ = mock_popen.call_args
+        command = args[0]
+        self.assertIn("-no-shell-escape", command)
+        self.assertIn("pdflatex", command)
+
+    @patch("cli.pdf.converter.subprocess.Popen")
+    def test_converter_pandoc_timeout(self, mock_popen):
+        process_mock = MagicMock()
+        process_mock.communicate.side_effect = [
+            subprocess.TimeoutExpired(cmd="pandoc", timeout=30),
+            (b"", b""),
+        ]
+        mock_popen.return_value = process_mock
+
+        converter = PDFConverter()
+
+        with patch.object(Path, "exists", return_value=False):
+            result = converter._compile_pandoc(Path("output.tex"), Path("output.pdf"), Path("."))
+
+        self.assertFalse(result)
+        process_mock.kill.assert_called_once()
+        process_mock.communicate.assert_any_call(timeout=30)
+
+    @patch("cli.pdf.converter.subprocess.Popen")
+    def test_converter_pandoc_arguments(self, mock_popen):
+        process_mock = MagicMock()
+        process_mock.communicate.return_value = (b"", b"")
+        process_mock.returncode = 0
+        mock_popen.return_value = process_mock
+
+        converter = PDFConverter()
+
+        with patch.object(Path, "exists", return_value=True):
+            converter._compile_pandoc(Path("output.tex"), Path("output.pdf"), Path("."))
+
+        args, _ = mock_popen.call_args
+        command = args[0]
+        self.assertIn("--pdf-engine-opt=-no-shell-escape", command)
+        self.assertIn("pandoc", command)
+
+    @patch("subprocess.Popen")
+    def test_cover_letter_pdflatex_timeout(self, mock_popen):
+        process_mock = MagicMock()
+        process_mock.communicate.side_effect = [
+            subprocess.TimeoutExpired(cmd="pdflatex", timeout=30),
+            (b"", b""),
+        ]
+        mock_popen.return_value = process_mock
+
+        # Setup env vars so CoverLetterGenerator initializes correctly
+        os.environ["ANTHROPIC_API_KEY"] = "mock"
+
+        # We need to mock yaml handler and config properly if we're not initializing fully,
+        # but _compile_pdf doesn't rely on `self` heavily. We can just instantiate it.
+        # It's better to patch the config to avoid errors
+        with patch("cli.generators.cover_letter_generator.Config") as mock_config, patch(
+            "cli.generators.cover_letter_generator.ResumeYAML"
+        ):
+
+            mock_cfg_inst = mock_config.return_value
+            mock_cfg_inst.ai_provider = "anthropic"
+            mock_cfg_inst.get.return_value = {}
+
+            generator = CoverLetterGenerator(yaml_path=Path("mock.yaml"), config=mock_cfg_inst)
+
+        with patch.object(Path, "exists", return_value=False):
+            result = generator._compile_pdf(Path("output.pdf"), "content")
+
+        # It fails and returns False
+        self.assertFalse(result)
+        process_mock.kill.assert_called_once()
+        process_mock.communicate.assert_any_call(timeout=30)
+
+    @patch("subprocess.Popen")
+    def test_cover_letter_pdflatex_arguments(self, mock_popen):
+        process_mock = MagicMock()
+        process_mock.communicate.return_value = (b"", b"")
+        process_mock.returncode = 0
+        mock_popen.return_value = process_mock
+
+        os.environ["ANTHROPIC_API_KEY"] = "mock"
+        with patch("cli.generators.cover_letter_generator.Config") as mock_config, patch(
+            "cli.generators.cover_letter_generator.ResumeYAML"
+        ):
+
+            mock_cfg_inst = mock_config.return_value
+            mock_cfg_inst.ai_provider = "anthropic"
+            mock_cfg_inst.get.return_value = {}
+
+            generator = CoverLetterGenerator(yaml_path=Path("mock.yaml"), config=mock_cfg_inst)
+
+        with patch.object(Path, "exists", return_value=True):
+            generator._compile_pdf(Path("output.pdf"), "content")
+
+        args, _ = mock_popen.call_args
+        command = args[0]
+
+        self.assertIn("-no-shell-escape", command)
+        self.assertIn("-interaction=nonstopmode", command)
+        self.assertIn("pdflatex", command)
+
 
 if __name__ == "__main__":
     unittest.main()