Implements --check-resolutions
#4302
Conversation
| if (opts.checkResolutions) { | ||
| const {locators} = await noLockfileResolver.getSatisfying(descriptor, resolvedDependencies, [finalResolution], {...resolveOptions, resolver: noLockfileResolver}); | ||
| if (!locators.find(locator => locator.locatorHash === finalResolution.locatorHash)) { | ||
| throw new ReportError(MessageName.RESOLUTION_MISMATCH, `Invalid resolution ${structUtils.prettyResolution(this.configuration, descriptor, finalResolution)}`); |
There was a problem hiding this comment.
I feel like the error message could include a more detailed explanation detailing what it means that the resolution is invalid, how it might've happened and how it could be fixed.
There was a problem hiding this comment.
I'd like to keep messages on one line if possible, so I was thinking to explain that in more details within the website's error page (especially since this error should never surface in regular situations).
There was a problem hiding this comment.
🤔 We can do it like that for now, but in the future, I feel like actually showing helpful advice directly (without having to navigate to the website) would be an important UX improvement. I really like the way you improved the project detection error message for example.
arcanis
force-pushed
the
mael/check-resolutions
branch
from
675e552 to
8ce30d9
Compare
|
appreciate this PR @arcanis 👍 handy when upgrading noticed v4's flagging old (v3) set resolutions if legit (not false positives), fixing one by one may take a while got a repro here, its a just read b34398a, lemme know if i've done smth strange, if a bug happy to raise the issue |
What's the problem this PR addresses?
If an attacker changes the
resolutionsfield in the lockfile, Yarn will take it relatively well and just assume the new resolution is the truth. As a result they can easily inject malicious code inside a large lockfile update, and they'll be very difficult to spot - even #4299 wouldn't protect against that, as Yarn would just refetch the compromised package resolution rather than the original one.How did you fix it?
The
getSatisfyingfunction (previously only used foryarn dedupe) will now be used to check that the resolution locators are valid for a given descriptor (for example[email protected]isn't a valid resolution forfoo@^1.0.0since they have different idents, but it is a valid resolution forbar@^1.0.0or evenfoo@npm:[email protected]).In order to allow for this, I had to change the signature of the
getSatisfyingfunction:patch:/exec:to compute their derivation)sortedproperty (required because sorting still doesn't make sense in many resolvers)More tests need to be written (I wrote a few to cover the main ones, ie Git and npm), but I'm not sure I'll write them all in this PR alone.
Checklist