FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(axios): updated axios version to fix critical vulnerability by joserodriguezjll · Pull Request #1387 · sendgrid/sendgrid-nodejs · GitHub

fix(axios): updated axios version to fix critical vulnerability - #1387

Open
joserodriguezjll wants to merge 1 commit into
sendgrid:mainfrom
joserodriguezjll:patch-1
Open

fix(axios): updated axios version to fix critical vulnerability#1387
joserodriguezjll wants to merge 1 commit into
sendgrid:mainfrom
joserodriguezjll:patch-1

Conversation

Copy link
Copy Markdown

Fixes

  • Updated axios version to fix critical vulnerability

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket.

# Fixes #

- Updated axios version to fix critical vulnerability

huineng commented Nov 10, 2023

Copy link
Copy Markdown

this is an important fix needed, is there any reason the tests are not running ?

Copy link
Copy Markdown
Author

this is an important fix needed, is there any reason the tests are not running ?

No idea, let's see if some of the product developers can check this issue and check the problem with the tests

Copy link
Copy Markdown
Contributor

this is an important fix needed, is there any reason the tests are not running ?

No idea, let's see if some of the product developers can check this issue and check the problem with the tests

Hi there,
I approved the tests run, this change is failing at build validation.
I have added this item to the backlog to be prioritised soon.

shrutiburman added the type: security known security issue label Nov 10, 2023

huineng commented Nov 13, 2023

Copy link
Copy Markdown

can someone look at this error, doesn't look like an axios problem but with the test itself
Thanks

Copy link
Copy Markdown
Contributor

Hi! I will be looking into this and include it in the next release

Copy link
Copy Markdown

Hi @tiwarishubham635, when will the next release be?

Copy link
Copy Markdown
Contributor

Hi @tiwarishubham635, when will the next release be?

This thursday

Copy link
Copy Markdown

Hi @tiwarishubham635, when will the next release be?

This thursday

Great, thanks!

selrond commented Nov 14, 2023
edited
Loading

Copy link
Copy Markdown

@tiwarishubham635 will other packages that use the @sendgrid/client (like @sendgrid/mail) be updated as well?

Copy link
Copy Markdown
Contributor

@tiwarishubham635 will other packages that use the @sendgrid/client (like @sendgrid/mail) be updated as well?

Yes, since the changes are in the package version of axios, all the affected places will be modified. I hope that answers your query.

ebickle commented Nov 15, 2023
edited
Loading

Copy link
Copy Markdown

@tiwarishubham635 The current version of Axios only supports Node.js 12.x and above:
https://github.com/axios/axios/blob/9588fcdec8aca45c3ba2f7968988a5d03f23168c/.github/workflows/ci.yml#L15

Node.js 6 is no longer in Long Term Support and has security vulnerabilities. My recommendation would be to treat this as a breaking change, drop support for older unsupported Node.js versions, then bump the major version.

You'll likely find that the these tests for Node.js 7, 8, and 10 will also fail the same way - they were skipped because the test for 6 failed.

codygordon mentioned this pull request Nov 15, 2023
8 tasks

Copy link
Copy Markdown
Contributor

@tiwarishubham635 The current version of Axios only supports Node.js 12.x and above: https://github.com/axios/axios/blob/9588fcdec8aca45c3ba2f7968988a5d03f23168c/.github/workflows/ci.yml#L15

Node.js 6 is no longer in Long Term Support and has security vulnerabilities. My recommendation would be to treat this as a breaking change, drop support for older unsupported Node.js versions, then bump the major version.

You'll likely find that the these tests for Node.js 7, 8, and 10 will also fail the same way - they were skipped because the test for 6 failed.

Yes, we have already identified this issue and are in the process of removing the older node versions. Once it is done, this PR will be merged

k725 commented Nov 21, 2023

Copy link
Copy Markdown

@tiwarishubham635 @shrutiburman "Thursday" is Not sure if it was last Thursday or this Thursday, but it looks like they did the old Node.js drop in #1390, so hopefully it will be merged soon.

Copy link
Copy Markdown
Contributor

Hi There,
Yes, the PR is in review, once it's merged the changes will be picked up in the upcoming release next week. We follow a bi-weekly release cadence.

Copy link
Copy Markdown
Contributor

Related PR: #1391

cemusta left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Copy link
Copy Markdown

@joserodriguezjll when you have a chance can you merge main into your branch?

Copy link
Copy Markdown

Okay, guys, maybe it will get updated in a few weeks.

tovbinm commented Dec 5, 2023

Copy link
Copy Markdown

What is the ETA to get this merged & released?

alsiola commented Dec 5, 2023

Copy link
Copy Markdown

@joserodriguezjll when you have a chance can you merge main into your branch?

This is a pretty critical issue for us and many others - perhaps the sendgrid team might want to drive this forwards without waiting for an external contributor?

Copy link
Copy Markdown

Na guys, main question is when will the npm package will get updated? I think the already merged/fixed the issue in Github repo, but not updated the npm module, btw is this correct npm module na? Because I am using this in my projects.

https://www.npmjs.com/package/@sendgrid/mail

shrutiburman commented Dec 5, 2023
edited
Loading

Copy link
Copy Markdown
Contributor

Hey everyone,
sendgrid-nodejs with axios update is released! v8.0.0.
It's available in npm.

Keeping this PR open for a couple days more, please reach out if you face any issues. Apologies for the delay.

shrutiburman added status: waiting for feedback waiting for feedback from the submitter dependencies pull requests that update a dependency file and removed status: ready for merge code deemed fit for merge labels Dec 5, 2023

Copy link
Copy Markdown

Let's gooooooooooooooooooooooooooooooooo.

jakebanks commented Jan 6, 2024
edited
Loading

Copy link
Copy Markdown

Has anyone had success using 8.0.0? I am seeing an issue as described here #1391 (comment) - axios3 is not a function

Also is there a reason Issues are disabled on this GitHub repo? I would like to raise this in the right place rather than in the comments section on several PRs.

Copy link
Copy Markdown

Has anyone had success using 8.0.0? I am seeing an issue as described here #1391 (comment) - axios3 is not a function

Also is there a reason Issues are disabled on this GitHub repo? I would like to raise this in the right place rather than in the comments section on several PRs.

Using it since the launch, never faced any issue till now mate.

Copy link
Copy Markdown

Has anyone had success using 8.0.0? I am seeing an issue as described here #1391 (comment) - axios3 is not a function

Also is there a reason Issues are disabled on this GitHub repo? I would like to raise this in the right place rather than in the comments section on several PRs.

I was thinking to shift from sendgrid soon, their service getting poor, & new clients onboarding issue too.

Copy link
Copy Markdown

This seems to be addressed in #1394, but three months to fix a vulnerability flagged as critical? Yikes.

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

Labels

dependencies pull requests that update a dependency file status: waiting for feedback waiting for feedback from the submitter type: security known security issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.


Back | FazBrowse Home | New Git URL