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

npm audit fix #10

Open
lourot opened this issue Feb 28, 2023 · 15 comments
Open

npm audit fix #10

lourot opened this issue Feb 28, 2023 · 15 comments
Labels
good first issue Good for newcomers

Comments

@lourot
Copy link
Member

lourot commented Feb 28, 2023

At the moment, NPM detects a high severity vulnerability in one of our dependency. However npm audit fix doesn't help:

$ npm audit fix

up to date, audited 774 packages in 3s

133 packages are looking for funding
  run `npm fund` for details

# npm audit report

marked  <=4.0.9
Severity: high
Marked ReDoS due to email addresses being evaluated in quadratic time - https://github.com/advisories/GHSA-xf5p-87ch-gxw2
Inefficient Regular Expression Complexity in marked - https://github.com/advisories/GHSA-5v2h-r2cx-5xgj
Inefficient Regular Expression Complexity in marked - https://github.com/advisories/GHSA-rrrm-qjm4-v8hf
fix available via `npm audit fix`
node_modules/marked

1 high severity vulnerability

To address all issues, run:
  npm audit fix
@lourot lourot added the good first issue Good for newcomers label Feb 28, 2023
@MarkovianPD
Copy link

I would like to take a crack at this!

@lourot
Copy link
Member Author

lourot commented Mar 2, 2023

Thank you @MarkovianPD !

@MarkovianPD
Copy link

MarkovianPD commented Mar 2, 2023

Thank you @MarkovianPD !

@AurelienLourot Absolutely! I'm new to contributions trying to get my foot in the door anyway possible and help out! I created a fork to mess around with it before your reply and from what I could dig up and conclude is that you'll want to upgrade npm package "marked" to version 0.6.2. I'm not sure if I can do that by editing the package.json version to the updated version and it will upgrade? It does also seem you're using a 0.3.X version and I assume you're using an older version for a reason and upgrading may not be the best solution? Any and all feedback would be appreciated!

@lourot
Copy link
Member Author

lourot commented Mar 2, 2023

marked is not one of our direct dependencies: https://github.com/Frameright/image-display-control-web-component/blob/main/image-display-control/package.json#L44

It must be a dependency of one of all these dependencies. So if we're using an old version, it's not on purpose from us, but it might be on purpose from one of these dependencies, I'm not sure. If pinning explicitly the latest marked in our package.json works, I'm happy with that workaround :) Thanks!

@MarkovianPD
Copy link

Very interesting! If you could explained the process of pinning the latest in your package.json I would be thankful and more than happy to get it done asap!

@lourot
Copy link
Member Author

lourot commented Mar 2, 2023

I was thinking of adding "marked": "^4.2.12" inside our devDependencies in package.json. I just tried it out, it didn't help:

$ rm -rf package-lock.json node_modules/
$ npm install
[...]

1 high severity vulnerability

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

$ npm audit fix
[...]
fix available via `npm audit fix`
node_modules/markdown-spellcheck/node_modules/marked

1 high severity vulnerability

To address all issues, run:
  npm audit fix

This is interesting however as it's showing me that it's our dependency markdown-spellcheck that is pulling an old marked. This project looks abandoned actually:

So contacting the maintainer to update their dependency to marked probably won't help. Are there newer/better tools for spellchecking markdown files these days, that we could use instead of markdown-spellcheck?

@MarkovianPD
Copy link

This is the only alternative I could find, https://socket.dev/npm/package/md-spell it says it is loosely based on lukeapage's node-markdown. I'm not sure this would accomplish exactly what you're looking for. Pertaining to the ugprade some users experiencing this issue in the past have commented that running npm upgrade solved the issue I would recommend at least trying this but as mentioned earlier there may be some issues if you're not wanting to upgrade specific packages. Multiple users responded to the recommendation stating that had worked for their situation.

@lourot
Copy link
Member Author

lourot commented Mar 2, 2023

npm upgrade doesn't help in my case. I think this is because markdown-spellcheck pins ^0.7.0: https://github.com/lukeapage/node-markdown-spellcheck/blob/master/package.json#L40

Which means "any version between 0.7.0 and 1", so this prevents us from getting the latest marked.

https://www.npmjs.com/package/md-spell has no link to a github repository, so I'm not sure where the code lives. There is a Tab Code there and I can see in package.json that they don't use marked anymore, which is good. They seem to use hunspell-spellchecker as a dependency, which hasn't been published for 8 years: https://www.npmjs.com/package/hunspell-spellchecker . I fear we would be switching to another abandoned project and run into the same kind of issues

@MarkovianPD
Copy link

I see, great catch I believe you're correct about jumping to another abandoned project. I believe I don't have sufficient knowledge to assist further with this but thank you for letting me have the chance to help you! I hope I can assist with other contributions in the future. If you find anything more on this npm audit and I can help I'll be ecstatic to help out!

@ilu
Copy link
Member

ilu commented Mar 2, 2023

@MarkovianPD @AurelienLourot

I think the cspell package could be used as an alternative to markdown-spellcheck:

It might require some configuring to have it spellcheck only the markdown files, but this guide might be helpful: https://tjaddison.com/blog/2021/02/spell-checking-your-markdown-blog-posts-with-cspell/

I don't have time right now to dig deeper, but happy to help you @MarkovianPD if you want to pursue this issue :)

@MarkovianPD
Copy link

@ilu Thank you for chiming in! I'll take a look at it when I can and see if any of it makes sense haha I'd love to crack the case and get it solved!

@MarkovianPD
Copy link

Thank you all for the help trying to get to the bottom of this! I've taken a read up on the cspell blog post and I believe this is beyond my expertise at the moment.

@ilu
Copy link
Member

ilu commented Mar 8, 2023

Thank you all for the help trying to get to the bottom of this! I've taken a read up on the cspell blog post and I believe this is beyond my expertise at the moment.

No worries @MarkovianPD, thank you for having a look. Please drop by again to see if any new issues would be suitable for you :)

lourot added a commit that referenced this issue Jun 28, 2023
as we are moving the documentation to
docs.frameright.io
@lourot
Copy link
Member Author

lourot commented Jun 28, 2023

FYI the spellchecker is now gone but we still have some npm audit issues. Keeping this open for now.

@lourot
Copy link
Member Author

lourot commented Jun 28, 2023

My mistake, I had forgotten to actually remove the dependency. Now the remaining npm audit issues are only moderate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants