Skip to content

Conversation

@Filtzkowski-Robert-bah
Copy link

@Filtzkowski-Robert-bah Filtzkowski-Robert-bah commented Jan 7, 2025

Fix vulnerability found in lower releases

Summary by CodeRabbit

  • Chores
    • Unpinned urllib3 version constraint in test requirements, allowing flexible version resolution.

Fix vulnerability found in lower releases
@NulledExceptions
Copy link

Bumping this up

@arbiv arbiv marked this pull request as draft November 11, 2025 08:57
@arbiv arbiv marked this pull request as ready for review November 11, 2025 08:58
@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

The urllib3 dependency in integration_tests/requirements.txt was unpinned by removing the exact version constraint (2.0.6), allowing any version of urllib3 to be installed during dependency resolution.

Changes

Cohort / File(s) Summary
Dependency unpinning
integration_tests/requirements.txt
Removed exact version constraint from urllib3==2.0.6 to urllib3, enabling flexible version resolution

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A pinned constraint springs free,
urllib3 roams wild with glee,
Versions dance without a bind,
Flexibility of heart and mind!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The PR title references fixing CVE-2023-45803 and CVE-2024-37891, but the actual change only unpins urllib3 without specifying which vulnerabilities are addressed or how they relate to the version change. Clarify the title to explain what vulnerability the urllib3 unpinning fixes, or provide more context about how this change addresses the referenced CVEs.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (2)
  • CVE-2023: Entity not found: Issue - Could not find referenced Issue.
  • CVE-2024: Entity not found: Issue - Could not find referenced 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2c599d1 and 2e39e1d.

📒 Files selected for processing (1)
  • integration_tests/requirements.txt (1 hunks)

pytest-html
filelock
urllib3==2.0.6
urllib3
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

❓ Verification inconclusive

Security fix is insufficient: unpinning does not guarantee CVE remediation.

CVE-2023-45803 requires urllib3 v1.26.18 or v2.0.7, and CVE-2024-37891 requires urllib3 version 1.26.19 or version 2.2.2. The original version 2.0.6 is vulnerable to both. However, removing the version pin entirely doesn't guarantee these fixes will be installed—any version of urllib3 (including vulnerable ones) could be resolved during pip install. This creates non-deterministic and potentially insecure dependency behavior across installations.

For proper security remediation in integration tests, specify a minimum version that contains both fixes (e.g., urllib3>=2.2.2) or document why a specific older version is acceptable.

Consider updating the requirement to:

-urllib3
+urllib3>=2.2.2

Or, if staying on the 2.0.x line is intentional, document that choice explicitly since the 2.x 2.0.7 version exists but CVE-2024-37891 is only fixed in 2.2.2.


I need to verify the CVE information before finalizing. Let me search for the specific vulnerability details.

Security fix is insufficient: unpinning does not guarantee CVE remediation.

CVE-2023-45803 has been addressed in versions 1.26.18 and 2.0.7, and CVE-2024-37891 requires either version 1.26.19 or version 2.2.2. The original pinned version 2.0.6 is vulnerable to both. However, removing the version pin entirely doesn't guarantee these fixes will be installed—any version of urllib3 (including vulnerable ones) could be resolved during pip install. This creates non-deterministic and potentially insecure dependency behavior across installations.

For proper security remediation in integration tests, specify a minimum version that contains both fixes (e.g., urllib3>=2.2.2), as the 2.0.x series does not have fixes for CVE-2024-37891.

Consider updating the requirement to:

-urllib3
+urllib3>=2.2.2
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
urllib3
urllib3>=2.2.2
🤖 Prompt for AI Agents
integration_tests/requirements.txt around line 6: the dependency urllib3 was
unpinned which does not ensure CVE fixes; update the requirement to enforce a
minimum safe version that includes both CVE fixes (e.g., change the line to
require urllib3>=2.2.2) so pip cannot resolve a vulnerable release and
installations are deterministic with the patched version.

@arbiv
Copy link
Contributor

arbiv commented Nov 19, 2025

Thanks for the contribution!
The fix was merged on #886

@arbiv arbiv closed this Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants