-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
[feat] yarn audit #5808
Comments
One thing that yarn could do, is improve on the concept. Perhaps yarn could add some interactivity, allowing the user to automatically update packages, as long as the suggested version is just a semver patch update. Also definitely useful would be different types of output (html, json, csv, or similar console outputs to test or lint runners etc.) I'm sure there are more ways yarn can add a bunch of value here. |
Would love yarn to add this. We're currently using snyk.io for vulnerability management but I've never been 100% happy with it as it always felt like something that should be more integrated. Was super excited when I heard that |
Right now both yarn and npm are able to autoresolve the lockfile conflicts
Yeah, that's the real problem |
|
Unfortunately, any new command addition is a breaking change for yarn, because of |
I don't see how. "scripts": {
"start": "echo 'this will override the default behavior of npm start'",
"audit": "echo 'my custom audit script will now be used'"
} |
Ok I see |
I haven't had time to actually implement anything here yet, so if anyone has free time, help is appreciated. Here are some notes from capturing the network traffic from npm: After all manifests metadata is loaded and before any .tgz packages are downloaded, npm makes a new request:
request body:
response headers:
response body:
The request includes the In addition to that, manually running the request headers:
request body:
response headers:
response body:
|
Discovered that the When that does not exist, the npm lockfile docs says "the sha1 in https://github.com/zkat/pacote/blob/ccc6e9094c2e872f09cc12ae966a0cbc1a570eed/lib/finalize-manifest.js#L100 leads me to believe that npm generates the missing integrity from the tarball, but watching network traffic, it has not yet downloaded the tarball 🤔 @zkat @imsnif can you provide any details as to how this works? (just asking is probably a lot easier than me hunting through the code 😄I appreciate any help!) |
The manifest json (eg. https://registry.npmjs.org/left-pad) should also have a |
@imsnif sorry for the lack of clarity on my part :) If you look at the registry response for
and the
so I'm wondering how it takes |
Ah, I think you're looking for this: https://github.com/zkat/ssri/blob/latest/index.js#L163-L173 Buffer.from('e0dd2120b49e1b724ce8d714c520822a9438576d', 'hex').toString('base64')
// 4N0hILSeG3JM6NcUxSCCKpQ4V20= |
Also - reading back now - if we need to talk to an |
Oh cool, yes that's probably something we want. Thanks for pointing it out 👍 |
https://github.com/zkat/pacote/blob/latest/lib/fetchers/registry/manifest.js#L129 This is where we fill this in for shasum-only packages |
And if a registry doesn't give either a shasum or an integrity field, then we calculate it off the tarball (ditto for other types that don't have this data, like remote tarball deps) |
I do think that was quite intentional on the npm team's part. |
@batjko Yeah uh, An interesting note is that we require a But tbh, in the end, you want this in Yarn itself. It's not gonna be as nice to be switching back and forth to npm just for the one feature and it won't have as good integration. npm@next can already auto-fix vulnerabilities, to boot, and that's not something we'd do for Yarn projects at all (because lockfile). |
(and it should tell you something that I'm showing up in this issue tracker and helping out, if you're concerned about Competition Issues) |
Just a quick update. I actually found some time to get started on this feature.
There is still a ton of work to do for the more in-depth reporting, but it prints a summary now at least. That's all, just wanted to let people know that this is at least in progress. |
@rally25rs excellent progress, I can not wait to test this new feature. |
@zkat To be clear, are you saying that even if Yarn adds an audit command, it would not add an audit fix command? The latter is super useful. I can't speak for anyone but me, but I'd switch back to NPM for improved security if Yarn doesn't plan to add an audit fix command. |
You might want to try Snyk. |
@DanielRuf Sorry, I didn't express myself well. I was referring to Yarn 1.12.3 being available in the Docker image of Node 8.14.0 – off topic ;-) |
@jverhoelen #6669 |
@alejandroiglesias you can use the |
You can make it a default by adding the following into your yarnrc:
|
@arcanis the |
Because the yarnrc files and the lockfiles share the same format, all yarn-generated configuration file have this header, but it's perfectly safe to edit the rc files (and even the lockfile, if you know what you're doing). It'll be fixed in the next major. |
@arcanis thanks! |
@arcanis one more question: is that alternatively achievable by using |
@alejandroiglesias it is achievable with
for add and install respectively |
@arcanis I added both CLI arguments in my project's
It now runs My yarn version is 1.13.0 |
Am I right in saying that there's no plan to add the |
Is there any chance we're able to use |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@shopglobal the latest stable version is v1.13.0 -- 1.3.2 is rather old at this point |
Current solution for fixiing audit results is like so:
why it works? Yarn respects |
Not anymore afaik. Also this is not really a solution. I think you have a different issue. |
That's really a smart and fast and convinient solution. |
That's never been the case - My guess is that by removing the lockfile OP simply forced Yarn to upgrade all their packages, hereby "fixing" the versions. You could do the same thing without calling |
the idea is using |
Or try snyk or use dependabot which can help here too (if you use a lockfile) =) |
Or sometime you can just remove I use this method to fix vulnerabilities on my project many times. 😁 |
@gluons in this case all packages will be updated and this is probably not what you want. Audit should update only effected packages or sub packages. |
Why not use |
From my experience, some deep dependencies still haven't been updated. (Some vulnerabilities are from deep nested dependencies.) If I remove |
Do you want to request a feature or report a bug?
feature
What is the current behavior?
npm added
audit
to warn about packages with known security issues. There was some conversation about this previously and one of the core npm folks said the API was likely to be open/public to pull this info. Therefore, yarn should be able to add this feature.What is the expected behavior?
yarn audit
command that mimicsnpm audit
Please mention your node.js, yarn and operating system version.
This would be a minor version bump, so likely target yarn v1.7.0 or v1.8.0 depending on timing.
This is probably too important to wait for v2.0.
The text was updated successfully, but these errors were encountered: