hardening(pdf): contain ordinary parser exceptions at validation boundary - #813
hardening(pdf): contain ordinary parser exceptions at validation boundary#813seonghobae wants to merge 8 commits into
Conversation
|
👋 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. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Current authority
develop@e06b1f3fb10903569124af011da213951e6e2473643e3f407ad71ce6f2b5c65f428b1a51620992aesrc/newsdom_api/main.pyplustests/test_pdf_validation_exception_boundary.pyCorrected finding
This change is parser-boundary robustness hardening. It is not evidence that a MEDIUM denial-of-service vulnerability has been fixed. Catching an exception cannot terminate a parser that is consuming excessive CPU or memory, and it does not address algorithmic-complexity or long-runtime vulnerabilities. Those require bounded execution/isolation and dependency-specific remediation.
The repository already uses
PdfReader(..., strict=True)and pins pypdf 6.15.0 on the protected base. Upstream's 2026read_until_whitespaceadvisory identifies<6.15.0as affected and 6.15.0 as the patched version; its malformed-startxrefadvisory affects<6.6.0and notes strict mode as a workaround. Neither advisory justifies calling this exception-catch delta a DoS fix.pypdf's own error documentation also warns that broken PDF files may cause exceptions beyond its documented exception classes. That supports a narrow external-library containment rationale: an ordinary
Exceptionraised while constructing/inspecting an untrusted PDF is translated into the existing fixed 415 response rather than escaping the upload-validation boundary.Deterministic contract
Focused regressions inject an ordinary third-party-style exception from
PdfReaderand require the fixed 415Unsupported Media Typeresponse without leaking parser internals. A complementary regression injectsKeyboardInterruptand requires it to propagate, proving that the boundary remainsException, notBaseException.The source comment was also repaired at the current head: it now describes the external-parser containment reason rather than claiming that an exception catch mitigates DoS/log exhaustion, and the now-unused
PdfReadErrorimport was removed. This does not constitute CPU/memory exhaustion evidence and must not be cited as such.Scope repair
The generated branch also changed
.jules/sentinel.mdinto a repository-wide rule and carried an unrelateduv.lockpypdf 6.15.0→6.17.0 update. Normal descendantc50387e6cf9fe59b35d570b65e86c6f4c509e75frestored both files to their exact protected-base blobs. No force push or destructive rebase was used.Any separate dependency upgrade must have its own causal advisory/release evidence, lock regeneration and compatibility tests rather than hitchhiking on this parser-boundary PR.
Exact-head evidence
Fresh runs have now materialized for
643e3f407...and are all non-terminal: tests34000513875, Security Scan34000513869, SAST Semgrep34000513925, CodeQL PR34000513914, codeql34000513888, clusterfuzzlite34000513949, container-image34000513851, scorecards34000513948. Predecessor results do not transfer.Keep Draft until terminal exact-head correctness/security/static-analysis evidence and current review. If the commercial requirement is hostile-PDF resource exhaustion resistance, follow with an isolation/time/CPU/RAM boundary and real/right-cleared hostile fixtures; do not use this exception translation as a substitute.