🛡️ Sentinel: [HIGH] Fix PDF compilation RCE vulnerabilities

[anchapin]

🚨 Severity: HIGH\n💡 Vulnerability: PDF compilation could allow RCE without -no-shell-escape and lead to zombie processes without timeout cleanups.\n🎯 Impact: Remote Code Execution or Denial of Service.\n🔧 Fix: Added appropriate flags and timeout cleanup.\n✅ Verification: New security tests added and passing.

---

PR created automatically by Jules for task 18071782307955844015 started by @anchapin

Summary by Sourcery

Harden PDF compilation paths against RCE and hanging processes by enforcing secure engine flags and subprocess timeouts.

Bug Fixes:

• Prevent remote code execution and zombie processes during PDF generation by securing pdflatex and pandoc invocations.

Enhancements:

• Enforce -no-shell-escape for pdflatex and --pdf-engine-opt=-no-shell-escape for pandoc in converter and cover letter PDF generation.
• Add 30-second timeouts with proper cleanup handling to all PDF compilation subprocesses.

Documentation:

• Extend Sentinel security notes with details about PDF compilation RCE and timeout handling requirements.

Tests:

• Add security-focused tests verifying pdflatex and pandoc are invoked with no-shell-escape flags and that timeouts trigger process cleanup in both converter and cover letter generator.

[google-labs-jules]

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[sourcery-ai]

Reviewer's Guide

This PR hardens all PDF compilation paths by enforcing no-shell-escape flags for pdflatex/pandoc, adding timeouts and cleanup for hanging subprocesses, and backfilling tests and sentinel documentation to cover these security guarantees.

Sequence diagram for hardened PDF compilation with no-shell-escape and timeouts

sequenceDiagram
    participant PdfCompiler
    participant Subprocess
    participant Pdflatex
    participant Pandoc

    PdfCompiler->>Subprocess: subprocess.Popen_pdflatex_no_shell_escape
    Subprocess->>Pdflatex: run_pdflatex
    alt pdflatex_finishes_before_timeout
        Pdflatex-->>Subprocess: process.communicate_timeout_30
        Subprocess-->>PdfCompiler: pdflatex_success_or_output_path_exists
    else pdflatex_hangs_timeout
        Subprocess-->>Pdflatex: process.kill
        Subprocess-->>PdfCompiler: pdflatex_timeout_failure
        PdfCompiler->>Subprocess: subprocess.Popen_pandoc_no_shell_escape
        Subprocess->>Pandoc: run_pandoc
        alt pandoc_finishes_before_timeout
            Pandoc-->>Subprocess: process.communicate_timeout_30
            Subprocess-->>PdfCompiler: pandoc_success_or_output_path_exists
        else pandoc_hangs_timeout
            Subprocess-->>Pandoc: process.kill
            Subprocess-->>PdfCompiler: pandoc_timeout_failure
        end
    end

Loading

File-Level Changes

Change	Details	Files
Harden PDFConverter’s pdflatex and pandoc compilation to prevent RCE and handle hung processes.	Add -no-shell-escape to the pdflatex invocation in the PDF converter. Add --pdf-engine-opt=-no-shell-escape to the pandoc invocation in the PDF converter while keeping xelatex as the engine. Wrap subprocess communicate() calls with a 30-second timeout for both pdflatex and pandoc. On timeout, kill the child process, re-run communicate() for cleanup, and return False from the compilation helpers.	cli/pdf/converter.py
Secure CoverLetterGenerator’s PDF compilation pipeline with shell-escape restrictions and timeout-based cleanup, including pandoc fallback.	Add -no-shell-escape to the pdflatex invocation when generating cover-letter PDFs. Add --pdf-engine-opt=-no-shell-escape to the pandoc fallback invocation while keeping --pdf-engine=xelatex. Introduce a 30-second timeout around pdflatex and pandoc communicate() calls in the cover-letter generator. On TimeoutExpired, kill the process, run a final communicate(), and mark PDF generation as failed rather than silently hanging.	cli/generators/cover_letter_generator.py
Backfill security-focused tests to assert flags, timeouts, and cleanup behavior for all PDF compilation paths.	Add tests for PDFConverter to verify timeout handling and presence of -no-shell-escape, -interaction=nonstopmode, and pandoc engine options. Add tests for CoverLetterGenerator to verify pdflatex and pandoc timeouts, flag usage, and fallback behavior when pdflatex is unavailable. Use patched subprocess.Popen and Path.exists to simulate success, timeout, and fallback scenarios without real process execution.	tests/test_pdf_security.py
Update Sentinel documentation to capture the newly addressed RCE and zombie process risks in PDF compilation.	Document the missing no-shell-escape flags and lack of timeouts as a high-severity vulnerability. Record learnings around consistently applying secure subprocess configuration across all PDF generation paths. Specify preventive measures: mandatory no-shell-escape flags and standardized timeout and cleanup patterns for subprocesses.	.jules/sentinel.md

Possibly linked issues

• #Security: Address bandit high severity findings in CI: Bandit likely flagged unsafe subprocess usage; this PR adds no-shell-escape and timeouts to those PDF compilation calls.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• The subprocess timeout and kill/cleanup logic is duplicated across multiple compilation paths; consider extracting a shared helper/wrapper for running pdflatex/pandoc with consistent flags, timeouts, and error handling.
• The 30-second timeout is hardcoded in several places; using a single configurable constant (or config option) would make it easier to tune and keep consistent across all PDF compilation routines.
• Given the security sensitivity of the no-shell-escape flags, it may be safer to centralize construction of pdflatex/pandoc command arguments so future additions to PDF generation can’t accidentally omit them.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The subprocess timeout and kill/cleanup logic is duplicated across multiple compilation paths; consider extracting a shared helper/wrapper for running pdflatex/pandoc with consistent flags, timeouts, and error handling.
- The 30-second timeout is hardcoded in several places; using a single configurable constant (or config option) would make it easier to tune and keep consistent across all PDF compilation routines.
- Given the security sensitivity of the no-shell-escape flags, it may be safer to centralize construction of pdflatex/pandoc command arguments so future additions to PDF generation can’t accidentally omit them.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}