chore(security): withdraw non-causal multipart DoS mitigation - #817
chore(security): withdraw non-causal multipart DoS mitigation#817seonghobae wants to merge 4 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
Changesparse 입력 검증
Estimated code review effort: 1 (매우 낮음) | ~5분 Merge Risk: ⚪ Minimal · up to The parse endpoint now rejects language and mode form values longer than 50 characters with HTTP 422 responses. The boundary behavior is covered by tests, with no remaining merge-readiness risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks 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 |
seonghobae
changed the title
1 participant
이 PR이 주장한
Form(max_length=50)기반 multipart 메모리 DoS 완화는 causal security fix가 아닙니다.Protected
develop@e06b1f3fb10903569124af011da213951e6e2473은 Starlette 1.3.1을 사용합니다. Starlette 공식 Requests 문서의 현재request.form()기본 계약은 non-file form field에max_part_size=1024*1024를 적용하며, 1.3.1 release note는FormParser에서도max_fields와max_part_size가 parser callback 단계에서 실제 enforce되도록 수정됐음을 명시합니다. FastAPIForm(max_length=50)validation은 이미 multipart parser가 field를 수신한 뒤의 application validation이므로,python-multipart가 무제한 text field를 메모리에 적재한다는 원래 RCA를 해결하지 않습니다.또한 product boundary는 이미
normalize_language()/normalize_mode()에서 closed supported set으로 검증되어 unsupported string을 422로 거부합니다. 임의의50자 상한은 별도 buyer/product contract 근거 없이 이 기존 UL을 중복하고, security severity를 뒷받침하지 않습니다.Concurrent descendant
a60d7522b0288a5e8176c5520937bb5e37b57821은 이 Form change에 무관한 pypdf 6.17 dependency/security delta까지 실었습니다.f35f4638d192611360c2c4060f934b6178f023bf에서 그 unrelated delta를 ordinary descendant로 제거해 원래 Form-only tree를 복구했고, 이어d5ae6d1e23e70153781e5978773a248ecc8aae53에서 causal review 결과에 따라 exact protected tree를 채택했습니다.Final compare: protected base 대비 ahead 4 / behind 0 / files=[]. 원래 commit과 intervening pypdf commit은 ancestry에 남아 있으며 force push/destructive rebase는 사용하지 않았습니다.
따라서 이 PR에는 보존할 유효 semantic/test/fixture/contract delta가 없습니다. 실제 hostile multipart resource-control gap을 추가로 다룰 경우에는 Starlette의 parser-level
max_part_size/max_fields, complete request의max_body_size또는RequestBodyLimitMiddleware, 그리고 PDF upload의 legitimate size contract를 함께 threat-model하고 representative hostile requests로 RED를 먼저 만들어야 합니다. 단순 Pydantic/FastAPI field validation을 parser-memory 증거로 사용하지 않습니다.References: Starlette Requests (
request.form(max_files, max_fields, max_part_size)), Starlette 1.3.1 release notes (#3329/#3331), Starlette request-body limits.Closed under the explicit no-valid-delta rule; no predecessor GREEN, scanner suppression, gate weakening, source-neutral retrigger, self-approval, or administrator bypass is used.