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

Yarn lock has security issues #2283

Closed
wolf99 opened this issue May 30, 2023 · 12 comments · Fixed by #2348 or #2352
Closed

Yarn lock has security issues #2283

wolf99 opened this issue May 30, 2023 · 12 comments · Fixed by #2348 or #2352

Comments

@wolf99
Copy link
Contributor

wolf99 commented May 30, 2023

The Yarn .lock file in this repo has security issues - see the Dependbot security alerts.

The dependencies there seem to be on som every old versions.
Im not really clear why a repo that doesn't havem JS content has a yarn file?
I don't know if there is a reason the dependencies are pinned to the major versions that they are pinned to?

But I am not a JS nor Yarn head so I am not the right person to fix this.

Maybe @ee7 or @ErikSchierboom knows?

@ErikSchierboom
Copy link
Member

Because we use nodejs for several tools: https://github.com/exercism/problem-specifications/blob/main/package.json

@NobbZ
Copy link
Member

NobbZ commented May 31, 2023

Perhaps it would still make sense to set up a dependabot, renovate, or any other tool to keep versions up2date?

@wolf99
Copy link
Contributor Author

wolf99 commented May 31, 2023

Because we use nodejs for several tools: https://github.com/exercism/problem-specifications/blob/main/package.json

But this is a JSON file, so data, not executable code.
Unless I missed it there is no executable JS in this repo. If tools from outside this repo are used within this repo, then shouldn't the lock file be stored with those tools rather than with the data?

Apologies if this is a silly question, I know nothing about JS. Just trying to get the security alerts resolved! 🙂

@wolf99
Copy link
Contributor Author

wolf99 commented May 31, 2023

Perhaps it would still make sense to set up a dependabot, renovate, or any other tool to keep versions up2date?

I think the existing dependabot would do this for us, except the lock file has several uses of ^, meaning that some dependencies are pinned to a specific major version. In the cases in question, those pinned major versions are now quite old.

I assume there was/is some functionality that depends on this pinning, but I don't know where or how to confirm if that is a correct guess, and if it is still relying on it today.

@SleeplessByte
Copy link
Member

#2283 (comment)
[...] lock file be stored with those tools rather than with the data?

No, we pull in specific versions of those tools, so that everyone interacting with this repo uses the same versions. That's by design because formatters, linters, etc. can have different output for different versions.

Because it's impossible to "package" the tool as a "binary" (we would need one including a node runtime for each architecture we run on, and it would be huge), the next best thing is to set up a package.json.

It is imperative we keep the pinning, but it is also true that they should periodically be updated, which is what you are asking for and that is the right way forward.

@wolf99
Copy link
Contributor Author

wolf99 commented Jun 1, 2023

Thanks for clarifying @SleeplessByte .

Then if updating the pinned dependencies is the way forward we'd need to understand what depends on those pinned versions.

So who knows which tools are depdent, and why, specifically, they are dependent on these pinned versions?

@SleeplessByte
Copy link
Member

In package.json you can see the tools (top level dependencies) we use and need. This is the avj cli and the prettier cli. If only the lock has issues and not the top levels, then it's a supply chain issue.

Generally updating them across the patch level version works, but almost always the issues are red herrings because there is actually no attack vector.

See: https://overreacted.io/npm-audit-broken-by-design/

@senekor
Copy link
Contributor

senekor commented Dec 7, 2023

I took a stab at updating prettier and avj-cli. Looks like both of them have breaking changes that are relevant for us.

avj-cli throws some errors I do not understand. I looked at the json-schema documentation and it's much more complicated than I would've expected.

prettier on the other hand reformats a bunch of stuff. Most things look harmless, but there are the custom blocks, e.g. ~~~~exercism/caution. These are reformatted to use three backticks.

I stopped investigating at this point, it would be more efficient if someone can do these updates who is more experienced with these tools.

Edit: Updating prettier isn't really relevant to this issue specifically. prettier isn't flagged for a security vulnerability.

@senekor senekor mentioned this issue Dec 9, 2023
@ErikSchierboom
Copy link
Member

prettier on the other hand reformats a bunch of stuff. Most things look harmless, but there are the custom blocks, e.g. ~~~~exercism/caution. These are reformatted to use three backticks.

We have a ruby script to fix those: https://github.com/exercism/problem-specifications/blob/f59cb2b5a38f4e0a1a65a121aac15839bcf7c6ab/.github/workflows/action-format.yml#L93C14-L93C14

@senekor
Copy link
Contributor

senekor commented Dec 12, 2023

Sorry for closing his prematurely. ajv-cli 5.0 still has a security issue flagged. Latest release was three years ago :( I'll check if there is another maintained validator we can switch to.

@senekor senekor reopened this Dec 12, 2023
@senekor
Copy link
Contributor

senekor commented Dec 12, 2023

The fix in ajv-cli has been ready to merge for almost a year, the project seems pretty much abandoned.

@wolf99
Copy link
Contributor Author

wolf99 commented Dec 13, 2023

Well done @senekor !! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants