Handle images and icons in submit blog post endpoint #113
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request adds image link handling to the blog post submission endpoint. When users provide HTTP(S) URLs for icon or image fields during blog post creation, the system automatically downloads the image content and saves it locally. A new utility function facilitates image downloading and wraps the result for Django ImageField compatibility, with corresponding logging throughout the process. Changes
Sequence DiagramsequenceDiagram
actor User
participant API as submit_blog_post<br/>Endpoint
participant BlogPost as BlogPost<br/>Object
participant Downloader as download_image_from_url
participant Storage as Storage<br/>(Filesystem)
User->>API: Submit blog post<br/>(with icon/image URLs)
API->>BlogPost: Create BlogPost instance
activate BlogPost
BlogPost-->>API: Object created
deactivate BlogPost
alt Icon or Image URL provided
API->>Downloader: download_image_from_url(url)
activate Downloader
Downloader->>Downloader: urlopen & fetch bytes
Downloader->>Downloader: Wrap in ContentFile
Downloader-->>API: ContentFile
deactivate Downloader
alt Download successful
API->>BlogPost: Save icon/image field
API->>Storage: Write file
Storage-->>API: File saved
API->>API: Log success
else Download fails
API->>API: Log error
end
end
API-->>User: Success response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Areas to review:
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Added image and icon URL handling to the submit_blog_post API endpoint. When creating a blog post, if icon or image URLs are provided, the endpoint now downloads them from external sources and saves them to Django storage.
- Created new
download_image_from_url()helper function incore/utils.pyto download images from URLs - Modified
submit_blog_post()endpoint to download and attach icon/image files when URLs are provided - Updated CHANGELOG to document the feature
Critical Security Issue: The implementation contains a Server-Side Request Forgery (SSRF) vulnerability that allows authenticated superusers to access internal network resources.
Other Issues:
- Missing timeout on network requests (can cause request handler to hang)
- Hardcoded
.pngfile extensions don't match actual image formats - Overly broad exception handling violates project style guide
Confidence Score: 1/5
- This PR introduces a critical SSRF vulnerability and should not be merged without security fixes
- The
download_image_from_urlfunction usesurlopenwithout URL validation, allowing potential access to internal network resources (localhost, private IPs, cloud metadata endpoints). While the endpoint requires superuser authentication, SSRF is still a serious security risk. Additional issues include missing timeouts and incorrect file extensions. core/utils.pyrequires immediate attention for SSRF vulnerability fix.core/api/views.pyneeds file extension handling correction.
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| core/utils.py | 1/5 | Added download_image_from_url helper function with SSRF vulnerability, missing timeout, and overly broad exception handling |
| core/api/views.py | 2/5 | Added icon and image URL download functionality to submit_blog_post endpoint with hardcoded PNG extensions |
Sequence Diagram
sequenceDiagram
participant Client
participant API as submit_blog_post API
participant BlogPost as BlogPost Model
participant Util as download_image_from_url
participant External as External Server
participant Storage as Django Storage
Client->>API: POST /blog-posts/submit<br/>(title, content, icon URL, image URL)
API->>BlogPost: create(title, description, slug, tags, content, status)
BlogPost-->>API: blog_post instance
alt icon URL provided
API->>API: strip() and validate URL starts with http/https
API->>Util: download_image_from_url(icon_url, "icon", blog_post.id)
Util->>External: urlopen(icon_url)
External-->>Util: image data
Util-->>API: ContentFile(image data)
API->>BlogPost: icon.save("blog-post-icon-{id}.png", content)
BlogPost->>Storage: save file to blog_post_icons/
end
alt image URL provided
API->>API: strip() and validate URL starts with http/https
API->>Util: download_image_from_url(image_url, "image", blog_post.id)
Util->>External: urlopen(image_url)
External-->>Util: image data
Util-->>API: ContentFile(image data)
API->>BlogPost: image.save("blog-post-image-{id}.png", content)
BlogPost->>Storage: save file to blog_post_images/
end
API-->>Client: BlogPostOut(status="success")
3 files reviewed, 5 comments
| instance_id=instance_id, | ||
| ) | ||
|
|
||
| image_response = urlopen(image_url) |
Contributor
There was a problem hiding this comment.
logic: SSRF (Server-Side Request Forgery) vulnerability: urlopen will fetch any URL without validation. An attacker with superuser API access could provide internal URLs like http://localhost:8080/admin or http://169.254.169.254/latest/meta-data/ (AWS metadata endpoint) to access internal resources.
Add URL validation to block private IP ranges and localhost:
Suggested change
| image_response = urlopen(image_url) | |
| # Validate URL to prevent SSRF attacks | |
| from urllib.parse import urlparse | |
| import ipaddress | |
| parsed = urlparse(image_url) | |
| if parsed.hostname: | |
| try: | |
| ip = ipaddress.ip_address(parsed.hostname) | |
| if ip.is_private or ip.is_loopback or ip.is_link_local: | |
| raise ValueError(f"Access to private/local IP addresses is not allowed") | |
| except ValueError as e: | |
| if "private" in str(e) or "loopback" in str(e) or "link_local" in str(e): | |
| raise | |
| # hostname is not an IP, check if it resolves to private IP | |
| import socket | |
| try: | |
| resolved_ip = socket.gethostbyname(parsed.hostname) | |
| ip = ipaddress.ip_address(resolved_ip) | |
| if ip.is_private or ip.is_loopback or ip.is_link_local: | |
| raise ValueError(f"Hostname resolves to private/local IP address") | |
| except socket.gaierror: | |
| pass # hostname doesn't resolve, let urlopen handle it | |
| image_response = urlopen(image_url, timeout=10) | |
| <details><summary>Prompt To Fix With AI</summary> | |
| `````markdown | |
| This is a comment left during a code review. | |
| Path: core/utils.py | |
| Line: 444:444 | |
| Comment: | |
| **logic:** SSRF (Server-Side Request Forgery) vulnerability: `urlopen` will fetch any URL without validation. An attacker with superuser API access could provide internal URLs like `http://localhost:8080/admin` or `http://169.254.169.254/latest/meta-data/` (AWS metadata endpoint) to access internal resources. | |
| Add URL validation to block private IP ranges and localhost: | |
| ```suggestion | |
| # Validate URL to prevent SSRF attacks | |
| from urllib.parse import urlparse | |
| import ipaddress | |
| parsed = urlparse(image_url) | |
| if parsed.hostname: | |
| try: | |
| ip = ipaddress.ip_address(parsed.hostname) | |
| if ip.is_private or ip.is_loopback or ip.is_link_local: | |
| raise ValueError(f"Access to private/local IP addresses is not allowed") | |
| except ValueError as e: | |
| if "private" in str(e) or "loopback" in str(e) or "link_local" in str(e): | |
| raise | |
| # hostname is not an IP, check if it resolves to private IP | |
| import socket | |
| try: | |
| resolved_ip = socket.gethostbyname(parsed.hostname) | |
| ip = ipaddress.ip_address(resolved_ip) | |
| if ip.is_private or ip.is_loopback or ip.is_link_local: | |
| raise ValueError(f"Hostname resolves to private/local IP address") | |
| except socket.gaierror: | |
| pass # hostname doesn't resolve, let urlopen handle it | |
| image_response = urlopen(image_url, timeout=10) | |
| How can I resolve this? If you propose a fix, please make it concise. |
|
|
||
| return image_content | ||
|
|
||
| except Exception as error: |
Contributor
There was a problem hiding this comment.
syntax: Broad exception handling violates style guide rule from .cursor/rules/backend-error-handling.mdc: "Use try-except blocks for specific errors, avoid excepting Exception". This catches and silently handles SSRF attacks, network errors, and other issues.
Suggested change
| except Exception as error: | |
| except (URLError, HTTPError, OSError) as error: | |
| **Context Used:** Context from `dashboard` - .cursor/rules/backend-error-handling.mdc ([source](https://app.greptile.com/review/custom-context?memory=9343d853-8bca-46ca-b37d-fd4327d3e3d2)) | |
| <details><summary>Prompt To Fix With AI</summary> | |
| `````markdown | |
| This is a comment left during a code review. | |
| Path: core/utils.py | |
| Line: 456:456 | |
| Comment: | |
| **syntax:** Broad exception handling violates style guide rule from `.cursor/rules/backend-error-handling.mdc`: "Use try-except blocks for specific errors, avoid excepting Exception". This catches and silently handles SSRF attacks, network errors, and other issues. | |
| ```suggestion | |
| except (URLError, HTTPError, OSError) as error: | |
| **Context Used:** Context from `dashboard` - .cursor/rules/backend-error-handling.mdc ([source](https://app.greptile.com/review/custom-context?memory=9343d853-8bca-46ca-b37d-fd4327d3e3d2)) | |
| How can I resolve this? If you propose a fix, please make it concise. |
| instance_id=instance_id, | ||
| ) | ||
|
|
||
| image_response = urlopen(image_url) |
Contributor
There was a problem hiding this comment.
logic: No timeout specified for urlopen. If the remote server is slow or unresponsive, this will block the request handler indefinitely.
Suggested change
| image_response = urlopen(image_url) | |
| image_response = urlopen(image_url, timeout=10) | |
| <details><summary>Prompt To Fix With AI</summary> | |
| `````markdown | |
| This is a comment left during a code review. | |
| Path: core/utils.py | |
| Line: 444:444 | |
| Comment: | |
| **logic:** No timeout specified for `urlopen`. If the remote server is slow or unresponsive, this will block the request handler indefinitely. | |
| ```suggestion | |
| image_response = urlopen(image_url, timeout=10) | |
| How can I resolve this? If you propose a fix, please make it concise. |
| image_url=icon_url, field_name="icon", instance_id=blog_post.id | ||
| ) | ||
| if icon_content: | ||
| filename = f"blog-post-icon-{blog_post.id}.png" |
Contributor
There was a problem hiding this comment.
logic: Hardcoded .png extension doesn't match the actual image format from the URL. If the URL points to a JPEG, GIF, or WebP, the file will have incorrect extension.
Suggested change
| filename = f"blog-post-icon-{blog_post.id}.png" | |
| # Extract extension from URL or default to png | |
| from urllib.parse import urlparse | |
| import os | |
| parsed_url = urlparse(icon_url) | |
| ext = os.path.splitext(parsed_url.path)[1] or '.png' | |
| filename = f"blog-post-icon-{blog_post.id}{ext}" | |
| <details><summary>Prompt To Fix With AI</summary> | |
| `````markdown | |
| This is a comment left during a code review. | |
| Path: core/api/views.py | |
| Line: 978:978 | |
| Comment: | |
| **logic:** Hardcoded `.png` extension doesn't match the actual image format from the URL. If the URL points to a JPEG, GIF, or WebP, the file will have incorrect extension. | |
| ```suggestion | |
| # Extract extension from URL or default to png | |
| from urllib.parse import urlparse | |
| import os | |
| parsed_url = urlparse(icon_url) | |
| ext = os.path.splitext(parsed_url.path)[1] or '.png' | |
| filename = f"blog-post-icon-{blog_post.id}{ext}" | |
| How can I resolve this? If you propose a fix, please make it concise. |
| image_url=image_url, field_name="image", instance_id=blog_post.id | ||
| ) | ||
| if image_content: | ||
| filename = f"blog-post-image-{blog_post.id}.png" |
Contributor
There was a problem hiding this comment.
logic: Same issue: hardcoded .png extension doesn't match the actual image format.
Suggested change
| filename = f"blog-post-image-{blog_post.id}.png" | |
| from urllib.parse import urlparse | |
| import os | |
| parsed_url = urlparse(image_url) | |
| ext = os.path.splitext(parsed_url.path)[1] or '.png' | |
| filename = f"blog-post-image-{blog_post.id}{ext}" | |
| <details><summary>Prompt To Fix With AI</summary> | |
| `````markdown | |
| This is a comment left during a code review. | |
| Path: core/api/views.py | |
| Line: 993:993 | |
| Comment: | |
| **logic:** Same issue: hardcoded `.png` extension doesn't match the actual image format. | |
| ```suggestion | |
| from urllib.parse import urlparse | |
| import os | |
| parsed_url = urlparse(image_url) | |
| ext = os.path.splitext(parsed_url.path)[1] or '.png' | |
| filename = f"blog-post-image-{blog_post.id}{ext}" | |
| How can I resolve this? If you propose a fix, please make it concise. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit