Skip to content

Fixes security vulnerability from url package dependency by updating url package version #3876

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

Closed
wants to merge 15 commits into from

Conversation

ugo96
Copy link

@ugo96 ugo96 commented May 27, 2025

Does your PR solve an issue?

Updates dependency on url package which has a transitive dependency on idna package where the vulnerability exists.

More details are available in the provided link.
GHSA-h97m-ww89-6jmq

Is this a breaking change?

No, the integration tests and build succeeds without any issues.
The updated package is also known to maintain backward compatibility without any breaking changes.

@aalhendi
Copy link

Hi. This is in fact a breaking change. However it seems like this should be OK to merge in 0.9.0.
The 0.9.0-dev branch (29b4fe7) has a MSRV of 1.86.0

Current (main):
From rust-toolchain.toml:

# Note: should NOT increase during a minor/patch release cycle
[toolchain]
channel = "1.78"
profile = "minimal"

From cargo-msrv:

Main (bab1b02):

Compatibility Check #7: Rust 1.78.0
  [OK]     Is compatible

Result:
   Considered (min … max):   Rust 0.11.0 … Rust 1.87.0
   Search method:            bisect
   MSRV:                     1.78.0
   Target:                   x86_64-unknown-linux-gnu

Your change:

Result:
   Considered (min … max):   Rust 0.11.0 … Rust 1.87.0
   Search method:            bisect
   MSRV:                     1.82.0
   Target:                   x86_64-unknown-linux-gnu

With output similar to the CI/CD pipeline failures.

error: rustc 1.78.0 is not supported by the following packages:
  [email protected] requires rustc 1.81
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.80.0
  [email protected] requires rustc 1.80.0
  [email protected] requires rustc 1.80.0
  [email protected] requires rustc 1.81
  [email protected] requires rustc 1.81
  [email protected] requires rustc 1.81
  [email protected] requires rustc 1.81
  [email protected] requires rustc 1.81
  [email protected] requires rustc 1.82
  [email protected] requires rustc 1.82
Either upgrade rustc or select compatible dependency versions with
`cargo update <name>@<current-ver> --precise <compatible-ver>`
where `<compatible-ver>` is the latest version supporting rustc 1.78.0

@paolobarbolini
Copy link
Contributor

It shouldn't be a breaking change if cargo update is run with the MSRV aware resolver.

The PR title seems misleading. If you did cargo update on all packages, then it should not read as if you only bumped url.

@ugo96
Copy link
Author

ugo96 commented May 27, 2025

Thanks, I initially followed up on the Rust discord server in the off-topic channel to ensure I'm following proper protocol.

I did run a cargo update on the package and used that to generate the updated Cargo.lock file, however, I did not confirm the MSRV version which as confirmed from the logs is causing the examples, checks, integration and CI tests to fail. I'm not as familiar with cargo as I'd like to be but this information is a great step in the right direction.

Apologies but this is my first PR in Rust and I would love to add documentation on how/if these checks can be validated locally[/in the forked repo] before PRs have been raised for the community to review which may reduce some of the friction that the community might be observing while a fresh set of eyes, like my own is onboarding.
I'm not sure if this is something that I may be experiencing due to a mistake on my end.

For next steps, I'll pull from the 0.9.0-dev branch and make the changes and confirm if that fixes the issue in this PR and re-raise it.

Edit: PR stems from my efforts to bump url package to resolve the linked dependency issue in the description of the PR, while I want to play a bigger role, this security vulnerability seemed like a good place to start since it required minimal and simple changes with great upsides for everyone

@ugo96 ugo96 changed the base branch from main to 0.9.0-dev May 27, 2025 18:15
@ugo96 ugo96 changed the title Fixes security vulnerability from url package dependency by updating package version Fixes security vulnerability from url package dependency by updating url package version May 27, 2025
@ugo96
Copy link
Author

ugo96 commented May 27, 2025

I overwrote multiple commits and force pushed those to my personal fork.
Not wanting to take credits for work that was not mine, I'm working on a draft PR before I raise it for review by the community.

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.

4 participants