fix(Generator): save uploads under a server-generated filename - #677
fix(Generator): save uploads under a server-generated filename#677harshita-singh12 wants to merge 4 commits into
Conversation
The upload path was built by joining the client-supplied filename with the upload folder, so a crafted name like ../../app.py could escape uploads/ and let file.save() overwrite arbitrary process-writable files (and os.remove() delete them afterwards). Derive the on-disk name from uuid4() on the server and keep only the extension, which already drives the text-extraction dispatch. File extensions outside the supported .txt/.pdf/.docx set are rejected before anything touches the disk, matching the previous behavior of returning empty content for unsupported types.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesSecure upload processing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change stores uploads under server-generated filenames and validates supported extensions before writing files; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue
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 |
Fixes #676
Problem
FileProcessor.process_file()built the on-disk path by joining the client-controlledfile.filenamewith the upload folder. A crafted filename such as../../app.pyescapesuploads/, sofile.save()can overwrite any process-writable file and the subsequentos.remove()can delete it.Fix
uuid.uuid4().hex), so the on-disk path can never escape the upload folder regardless of what the client sends..txt,.pdf,.docx). Anything else returns""before anything is written to disk, which matches the previous outcome for unsupported types (/uploadresponds 400).file.filename.Testing
Validated the
process_filelogic directly (module import stubbed for heavy ML deps):.txtupload round-trips and the saved path is inside the upload folder with a uuid name;../../evil.txtand../server.docxare saved inside the upload folder (no file created outside it);.md) saves nothing and returns""(same/upload400 response as before).Summary by CodeRabbit