π‘οΈ Sentinel: [CRITICAL] Fix LaTeX RCE and DoS vulnerabilities#393
π‘οΈ Sentinel: [CRITICAL] Fix LaTeX RCE and DoS vulnerabilities#393anchapin wants to merge 2 commits into
Conversation
* Added `-no-shell-escape` flag to pdflatex subprocess calls. * Added `--pdf-engine-opt=-no-shell-escape` flag to pandoc subprocess calls. * Added 30-second timeouts to mitigate DoS via compilation hangs. * Properly kill and clean up zombie processes on TimeoutExpired. * Added comprehensive tests in `tests/test_pdf_security.py` to verify flags and timeout behavior. Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
|
π Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a π emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's GuideThis PR hardens LaTeX-based PDF generation against RCE and DoS by adding no-shell-escape flags and strict timeouts to pdflatex/pandoc subprocess calls, and introduces tests to verify secure behavior and document the sentinel finding. Sequence diagram for secured LaTeX PDF compilation with timeout and no shell escapesequenceDiagram
actor User
participant CoverLetterGenerator
participant PdflatexProcess
User->>CoverLetterGenerator: generate_cover_letter()
CoverLetterGenerator->>CoverLetterGenerator: _compile_pdf(output_path, tex_content)
CoverLetterGenerator->>PdflatexProcess: subprocess.Popen(["pdflatex", "-interaction=nonstopmode", "-no-shell-escape", tex_path.name])
CoverLetterGenerator->>PdflatexProcess: communicate(timeout=30)
alt subprocess.TimeoutExpired
CoverLetterGenerator->>PdflatexProcess: kill()
CoverLetterGenerator->>PdflatexProcess: communicate()
CoverLetterGenerator-->>User: return False (PDF generation aborted)
else process.returncode == 0 or output_path.exists()
CoverLetterGenerator-->>User: return True (PDF generated securely)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The timeout handling logic for LaTeX/Pandoc subprocesses is duplicated in multiple methods; consider extracting a small helper to encapsulate the
Popen+communicate(timeout)+TimeoutExpired/kill pattern for consistency and easier maintenance. - The 30-second timeout is currently hard-coded in several places; it may be worth making this configurable (e.g., via a constant or config option) so it can be tuned without code changes and kept consistent across all PDF compilation paths.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The timeout handling logic for LaTeX/Pandoc subprocesses is duplicated in multiple methods; consider extracting a small helper to encapsulate the `Popen` + `communicate(timeout)` + `TimeoutExpired`/kill pattern for consistency and easier maintenance.
- The 30-second timeout is currently hard-coded in several places; it may be worth making this configurable (e.g., via a constant or config option) so it can be tuned without code changes and kept consistent across all PDF compilation paths.Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
* Added `-no-shell-escape` flag to pdflatex subprocess calls. * Added `--pdf-engine-opt=-no-shell-escape` flag to pandoc subprocess calls. * Added 30-second timeouts to mitigate DoS via compilation hangs. * Properly kill and clean up zombie processes on TimeoutExpired. * Added comprehensive tests in `tests/test_pdf_security.py` to verify flags and timeout behavior. Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
π¨ Severity: CRITICAL
π‘ Vulnerability: Missing -no-shell-escape and timeout in PDF compilation
π― Impact: Remote Code Execution (RCE) via LaTeX injections and Denial of Service (DoS) via compilation hangs
π§ Fix: Added -no-shell-escape, --pdf-engine-opt=-no-shell-escape, and timeout=30 to pdflatex and pandoc subprocess calls. Properly handle subprocess.TimeoutExpired
β Verification: Run python -m pytest tests/test_pdf_security.py and full test suite
PR created automatically by Jules for task 8216287924626519256 started by @anchapin
Summary by Sourcery
Harden LaTeX-based PDF generation against RCE and DoS by enforcing secure subprocess flags and timeouts across converters and cover letter generation.
Bug Fixes:
Enhancements: