Skip to content
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

Bump @azure/identity to 4 to solve CVE-2024-35255 #1343

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

carboneater
Copy link

CVE-2024-35255: GHSA-m5vv-6r4h-3vj9

Also tweak a few test processes

@carboneater
Copy link
Author

@microsoft-github-policy-service agree company="Desjardins"

@JacksonWeber JacksonWeber self-requested a review July 19, 2024 21:28
@JacksonWeber JacksonWeber reopened this Aug 6, 2024
@JacksonWeber
Copy link
Contributor

@carboneater Doesn't look like this fully fixes the backCompatTests as I'm getting a host of type errors from both the azure SDK and OpenTelemetry when running them. Are you seeing the same behavior?

@carboneater
Copy link
Author

@JacksonWeber
I have the same erratic behaviour you do.

I tried fixing that folder in hopes of coaxing the CI to work...

But at some point, I had to give up.

Looking back at the code of the test, looks like the back compatibility test attempts to build the package using @types/node@8 and the "latest" test attempts the same using @types/node@10

Given that the current version of Node is now 22, and versions prior to 18 are deprecated, I'm not sure how we'd want to solve this conundrum.

Also, protobufjs depends on @types/node@>=13.7.0, which makes for a double types definition in the NPM tree.
And probably the root cause of all the issues we're seeing

I see a few way to go forwards:

  1. Dismiss this PR and ignore the error. I would advise against it.
  2. Remove these failing tests from the project & doc entirely. Again, not the best outcome, but better than carrying around broken/outdated tests.
  3. Re-Scope Supported versions and update tests to Node LTS.
    Digging in the package-lock.json, the package is using @azure/* packages which declare a {"engines": ">=18.0.0"}
    If we want to bring these tests back to life, we should probably limit back compatibility tests to Node versions under LTS.
    So 18, 20 & 22 as it stands.
    (This PR should probably be dismissed into an issue until it is successfully resolved.)

I would strongly advise going with the latter option.
And I'd be willing to help implementing these changes should you decide to do so.

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.

2 participants