🛡️ Sentinel: [CRITICAL/HIGH] Fix Remote Code Execution in PDF compilation#222
🛡️ Sentinel: [CRITICAL/HIGH] Fix Remote Code Execution in PDF compilation#222
Conversation
…tion 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 GuideAdds defense-in-depth against LaTeX-based RCE/DoS in cover letter PDF generation by disabling shell escapes for pdflatex/pandoc and enforcing compilation timeouts, and records the change in the Sentinel security journal. Sequence diagram for updated PDF compilation with secure subprocess handlingsequenceDiagram
actor User
participant CLI as CoverLetterGenerator
participant Pdflatex as PdflatexProcess
participant Pandoc as PandocProcess
User->>CLI: generate_cover_letter_pdf
CLI->>CLI: _compile_pdf(output_path, tex_content)
CLI->>Pdflatex: Popen([pdflatex, -interaction=nonstopmode, -no-shell-escape, tex_path])
CLI->>Pdflatex: communicate(timeout=30)
alt Pdflatex completes in time
Pdflatex-->>CLI: stdout, stderr, returncode
alt Pdflatex success or output_path exists
CLI->>CLI: pdf_created = True
CLI-->>User: success
else Pdflatex failure
CLI->>Pandoc: Popen([pandoc, tex_path, -o, output_path, --pdf-engine=xelatex, --pdf-engine-opt=-no-shell-escape])
CLI->>Pandoc: communicate(timeout=30)
alt Pandoc completes in time
Pandoc-->>CLI: stdout, stderr, returncode
alt Pandoc success or output_path exists
CLI->>CLI: pdf_created = True
CLI-->>User: success
else Pandoc failure
CLI-->>User: failure
end
else Pandoc timeout
CLI->>Pandoc: kill()
Pandoc-->>CLI: stdout, stderr
CLI-->>User: failure
end
end
else Pdflatex timeout
CLI->>Pdflatex: kill()
Pdflatex-->>CLI: stdout, stderr
CLI->>Pandoc: Popen([pandoc, tex_path, -o, output_path, --pdf-engine=xelatex, --pdf-engine-opt=-no-shell-escape])
CLI->>Pandoc: communicate(timeout=30)
alt Pandoc completes in time
Pandoc-->>CLI: stdout, stderr, returncode
alt Pandoc success or output_path exists
CLI->>CLI: pdf_created = True
CLI-->>User: success
else Pandoc failure
CLI-->>User: failure
end
else Pandoc timeout
CLI->>Pandoc: kill()
Pandoc-->>CLI: stdout, stderr
CLI-->>User: failure
end
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 30-second timeout value is currently hardcoded in two places; consider extracting this into a single constant or configuration option so it’s easier to tune or adjust per environment.
- The subprocess invocation and timeout-handling logic for
pdflatexandpandocis nearly identical; you could factor this into a small helper function to reduce duplication and keep the error-handling behavior consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The 30-second timeout value is currently hardcoded in two places; consider extracting this into a single constant or configuration option so it’s easier to tune or adjust per environment.
- The subprocess invocation and timeout-handling logic for `pdflatex` and `pandoc` is nearly identical; you could factor this into a small helper function to reduce duplication and keep the error-handling behavior consistent.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
🚨 Severity: CRITICAL
💡 Vulnerability: Subprocess
pdflatexandpandocexecuted without-no-shell-escapeflag and missing timeout.🎯 Impact: Malicious inputs in Cover Letters or templates could execute arbitrary shell code via LaTeX
\write18capabilities or cause Denial of Service by infinitely compiling.🔧 Fix: Add
-no-shell-escapeflag and process timeouts to PDF compilation logic inCoverLetterGenerator._compile_pdf.✅ Verification: Ran test suite locally using
python -m pytest tests/test_cover_letter_generator.pyand validated passing tests. Included updated Sentinel Journal entry.PR created automatically by Jules for task 1606002574241256488 started by @anchapin
Summary by Sourcery
Harden PDF compilation for cover letters against remote code execution and runaway LaTeX processes.
Bug Fixes:
Documentation: