-
Notifications
You must be signed in to change notification settings - Fork 289
[ROB-2886] - urllib cve patches #1981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughPython requirement raised to >=3.10,<3.12; multiple dependency versions updated and Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pyproject.toml (2)
62-62: Verify exact version pinning is intentional.The prometrix version constraint changed from
"^0.2.5"(allowing compatible minor/patch updates) to"0.2.9"(exact version). This removes semver flexibility and may prevent automatic security patches. Was this intentional, or should it be"^0.2.9"?
79-79: LGTM: h2 dependency added for CVE remediation.The h2 dependency addition is appropriate for urllib3 2.x. The comment references a Dependabot security alert. Consider updating the comment to specify which CVE this addresses (if it's one of CVE-2025-66418/CVE-2025-66471 or a separate h2-specific CVE) for better documentation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run_tests
- GitHub Check: run_tests
🔇 Additional comments (1)
pyproject.toml (1)
75-75: urllib3 2.6.2 upgrade correctly addresses CVE-2025-66418 and CVE-2025-66471.The upgrade from urllib3 1.26.20 to 2.6.2 properly fixes both CVEs (DoS vulnerabilities via decompression chains, affecting all versions < 2.6.0). Verification confirms no breaking changes will impact this codebase:
- No direct urllib3 imports detected
- Indirect dependencies (requests 2.32.3, botocore 1.42.19, httpx 0.27.2) all support urllib3 2.x
- Version constraint ^2.6.2 is secure and appropriate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
playbooks/robusta_playbooks/krr.py (2)
5-5: Fix import error: NONE doesn't exist in pickle module.The
picklemodule does not exportNONE. This will cause anImportErrorwhen the module is loaded.🔎 Proposed fix
-from pickle import NONEThen update line 451 to use the built-in
None:- _publish_krr_finding(event=event,krr_json=params.result, scan_id=params.scan_id, start_time=params.start_time, metadata=metadata, timeout=None, config_json=NONE) + _publish_krr_finding(event=event,krr_json=params.result, scan_id=params.scan_id, start_time=params.start_time, metadata=metadata, timeout=None, config_json=None)
408-408: Fix type annotation syntax error.The parameter
config_json = Optional[str]has incorrect syntax. Type annotations should use:not=. This should be eitherconfig_json: Optional[str]orconfig_json: Optional[str] = None.🔎 Proposed fix
-def _publish_krr_finding(event: ExecutionBaseEvent, krr_json: Dict[str, Any],scan_id: str, start_time: str, metadata: Dict[str, Any], timeout: Optional[str] = None, config_json = Optional[str]): +def _publish_krr_finding(event: ExecutionBaseEvent, krr_json: Dict[str, Any], scan_id: str, start_time: str, metadata: Dict[str, Any], timeout: Optional[str] = None, config_json: Optional[str] = None):Note: Also fixed the missing space after the comma before
scan_id.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
playbooks/robusta_playbooks/krr.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run_tests
- GitHub Check: run_tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test_robusta.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: run_tests
- GitHub Check: run_tests
CVE-2025-66418
CVE-2025-66471
Needed to use latest version of cli for package compatibility
We removed --disable-cloud-routing from the cli 5 months ago so that had to be removed
KRR version bump