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

Scc 4043 #19

Merged
merged 3 commits into from
Apr 10, 2024
Merged

Scc 4043 #19

merged 3 commits into from
Apr 10, 2024

Conversation

danamansana
Copy link
Contributor

  • Bump Node version to 20
  • Associated changes to CHANGELOG.md, README.md, package.json, package-lock.json

@danamansana danamansana requested a review from nonword April 8, 2024 15:00
Copy link
Member

@nonword nonword left a comment

Choose a reason for hiding this comment

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

I see a failing test locally. Can you add a minimal .travis.yml(or gh actions) to catch that? It also looks like there are some npm audit notices.

@danamansana
Copy link
Contributor Author

Which test do you see failing locally? I don't see any failures. As for npm audit, this is what I get:

# npm audit report

axios  0.8.1 - 0.27.2
Severity: moderate
Axios Cross-Site Request Forgery Vulnerability - https://github.com/advisories/GHSA-wf5p-g6vw-rhxx
No fix available
node_modules/axios
node_modules/axios-concurrency/node_modules/axios
  axios-concurrency  *
  Depends on vulnerable versions of axios
  node_modules/axios-concurrency

2 moderate severity vulnerabilities

Some issues need review, and may require choosing
a different dependency.

Not sure if that can be fixed without switching out axios-concurrency. I can take a look to see if that is doable.

Copy link
Member

@nonword nonword left a comment

Choose a reason for hiding this comment

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

Not sure if that can be fixed without switching out axios-concurrency. I can take a look to see if that is doable.

I took a look and it does seem like axios-concurrency has an unavoidable moderate vuln. Our use of axios-concurrency is kind of key. I don't see support for limiting concurrency in native fetch. So I'll ticket coming back to this and using another limiting method like p-limit (and maybe migrating off axios).

@danamansana danamansana merged commit 313540e into main Apr 10, 2024
1 check passed
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.

None yet

2 participants