fix seed SSRF - #942
Open
ZohaibHassan16 wants to merge 3 commits into
Open
Conversation
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoHarden SeedDataManager API loading with SSRF-guarded requests
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
🔒 Security Scan ResultsSafety — dependency vulnerabilities✅ No findings. Bandit — HIGH-severity code issues✅ No findings. Semgrep — static analysis patternsFound 55. Show all findings
This security scan runs automatically on source-code PRs and bi-weekly (skipped for doc/markdown-only changes). 📊 Security Policy: CI fails on Safety vulnerabilities and Bandit HIGH-severity findings. Semgrep findings above are informational and do not block merge. |
Code Review by Qodo
1. No private-IP escape hatch
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Collaborator
Author
|
Qodo bot's finding is irrelevant to this PR and that is intentional for this PR.
|
Understood. The current PR review index contains no active findings, so there is nothing here to dismiss. Treating |
🔒 Security Scan ResultsSafety — dependency vulnerabilities✅ No findings. Bandit — HIGH-severity code issues✅ No findings. Semgrep — static analysis patternsFound 55. Show all findings
This security scan runs automatically on source-code PRs and bi-weekly (skipped for doc/markdown-only changes). 📊 Security Policy: CI fails on Safety vulnerabilities and Bandit HIGH-severity findings. Semgrep findings above are informational and do not block merge. |
🔒 Security Scan ResultsSafety — dependency vulnerabilities✅ No findings. Bandit — HIGH-severity code issues✅ No findings. Semgrep — static analysis patternsFound 55. Show all findings
This security scan runs automatically on source-code PRs and bi-weekly (skipped for doc/markdown-only changes). 📊 Security Policy: CI fails on Safety vulnerabilities and Bandit HIGH-severity findings. Semgrep findings above are informational and do not block merge. |
Sameer6305
approved these changes
Aug 12, 2026
Sameer6305
left a comment
Sameer6305
left a comment
Collaborator
There was a problem hiding this comment.
Verified the fix end-to-end against upstream/main. The original SSRF vulnerability is confirmed, and the change to request_with_ssrf_guard() correctly closes the affected path.
I also verified the redirect protection, DNS/IP validation, blocked internal addresses, and relevant error handling. The focused tests and existing SSRF coverage pass successfully, with no blocking regressions found.
I found a few unrelated pre-existing issues during the review as well:
Authorizationheaders can be forwarded across cross-origin redirects.- The caller-provided
headersdictionary can be mutated when addingAuthorization. - There is a now-unnecessary
except (ImportError, OSError)around the old lazyrequestsimport.
These are outside the scope of this PR, so I’ll file separate issues for them.
The PR is correct and approved from my side.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Fixed SSRF issue in
SeedDataManager.load_from_api().Previously it was directly using
requests.get()with the user provided API URL. Now it uses the existingrequest_with_ssrf_guard()helper.Changes
requests.get()call withrequest_with_ssrf_guard()Testing