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

Upgrade vulnerable dependencies fixing npm audit reports #145

Merged
merged 9 commits into from
Nov 6, 2023

Conversation

sudheesh001
Copy link
Contributor

  • The current base branch of the webservice repository contains 10 dependencies with vulnerabilities
  • This patch upgrades the dependencies keeping compatibility and fixes the remaining dependencies
  • npm audit shows the result as found 0 vulnerabilities

Signed-off-by: Sudheesh Singanamalla sudheesh@cs.washington.edu

@danyalaytekin
Copy link
Member

danyalaytekin commented Nov 3, 2023

Hi @sudheesh001, thank you for this contribution, and sorry it's taken a while to review it.

This repo's assumptions had grown stale, as they had done in a few sibling repos, so I've made a few further changes to bring it closer to passing, such as using ubuntu-20.04 instead of ubuntu-latest until we can do a full dependency update for pa11y-webservice@5, and I've also restored the lockfile to v1 as it had been previously, since this'll probably go into a minor release first - hope that's ok. I think I've hit a wall now, where hapi@21 is failing here for Node 12 but hapi@20 is failing for Node 16, each of which we need to support in pa11y-webservice@4. Hopefully I'm wrong though and it's a flake or something else innocuous - I'll look into this again. Thanks again 🙂

@danyalaytekin
Copy link
Member

Aha, I'd reverted hapi too far! Again, assuming this successful build isn't also a flake.

@sudheesh001
Copy link
Contributor Author

@danyalaytekin Thank you for following up on this one! This looks great, I'm looking forward to seeing this merged. It does look like github actions indicates a few warnings. Do they need to be resolved before this merge goes through?

@danyalaytekin
Copy link
Member

Hi @sudheesh001, the warnings are about the structure of functions, but neither of us have touched the relevant functions here, so the warnings don't indicate a regression. I think we're good.

Copy link
Member

@josebolos josebolos left a comment

Choose a reason for hiding this comment

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

Excellent! Thanks all!

Copy link
Member

@hollsk hollsk left a comment

Choose a reason for hiding this comment

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

co-signed! Thank you very much, @sudheesh001 and @danyalaytekin 🙏

@danyalaytekin danyalaytekin merged commit eb1ae50 into pa11y:main Nov 6, 2023
3 checks passed
@sudheesh001 sudheesh001 deleted the dependency_fixes branch November 6, 2023 16:22
@danyalaytekin danyalaytekin added this to the 4.2 milestone Nov 7, 2023
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