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

Add outdated and audit commands #109

Merged
merged 1 commit into from
May 22, 2022

Conversation

cover
Copy link
Contributor

@cover cover commented Mar 7, 2022

This adds two new commands to Importmap: ./bin/importmap audit and ./bin/importmap outdated.

Both commands use npm's endpoints to fetch the related data.

While audit has an endpoint where they accept multiple packages, I couldn't find any to retrieve the versions of multiple packages in order to decrease the requests for the outdated check; and thus it's doing one request for each package.

I initially used a much simpler regex to extract the name and version (/^pin "([^"]*)".*@(\d+\.\d+\.\d+(?:[^\/\s]+)?).*$/), but then I discovered that some packages are loaded from inside other packages, and thus I couldn't rely on the name provided to the pin method. For a better maintainability I've kept the two regex separate: 1 for remote dependencies, and 1 for local dependencies. The | would also have the nice addition of "uniq" them.

Such dependencies that are downloaded locally might still be affected by this, and there is no reference to the original package name. One option here could be to change the current comment from just # @version to something like # from url, that way it'd include the version, but also the source/package name, and that point we could also have one single regex. What do you think? (I haven't done that change yet)

The current regex would also take care of versions like 2.0.0-beta.19 or similar (I've run a check on all versions from ~350k npm packages, and they were all covered by this).

It relies on the current structure of those urls, which might be risky on the long term (if they ever change it); an alternative solution could be to add a custom comment for this purpose (always, or just when package name is different), but then we'd have to take care of existing importmap files without that comment.

You can check here some sample data, it should cover all cases 🤞

There could be some edge cases, for example if someone adds a comment similar to the local one to a remote dependency, or uses a different provider. I don't know if we want to support for those as well.

Audit

+-------------+----------+---------------------+--------------------------------------------------------+
| Package     | Severity | Vulnerable versions | Vulnerability                                          |
+-------------+----------+---------------------+--------------------------------------------------------+
| glob-parent | high     | <5.1.2              | Regular expression denial of service                   |
| is-svg      | high     | >=2.1.0 <4.3.0      | ReDOS in IS-SVG                                        |
| is-svg      | high     | >=2.1.0 <4.2.2      | Regular Expression Denial of Service (ReDoS)           |
| lodash      | critical | <4.17.12            | Prototype Pollution in lodash                          |
| lodash      | high     | <4.17.21            | Command Injection in lodash                            |
| lodash      | high     | <4.17.19            | Prototype Pollution in lodash                          |
| lodash      | high     | <4.17.11            | Prototype Pollution in lodash                          |
| lodash      | low      | <4.17.5             | Prototype Pollution in lodash                          |
| lodash      | moderate | <4.17.21            | Regular Expression Denial of Service (ReDoS) in lodash |
| lodash      | moderate | <4.17.11            | Prototype pollution in lodash                          |
| nth-check   | moderate | <2.0.1              | Inefficient Regular Expression Complexity in nth-check |
+-------------+----------+---------------------+--------------------------------------------------------+
  11 vulnerabilities found: 6 high, 3 moderate, 1 low, 1 critical

Outdated

+-----------------+---------------+---------------+
| Package         | Current       | Latest        |
+-----------------+---------------+---------------+
| @jspm/core      | 2.0.0-beta.18 | 2.0.0-beta.19 |
| @jspm/core      | 2.0.0-beta.2  | 2.0.0-beta.19 |
| aaaasssstimulus | 2.0.0         | Not found     |
| glob-parent     | 3.1.0         | 6.0.2         |
| is-glob         | 3.1.0         | 4.0.3         |
| is-svg          | 3.0.0         | 4.3.2         |
| lodash          | 4.17.1        | 4.17.21       |
| nth-check       | 1.0.0         | 2.0.1         |
| react           | 16.0.0        | 17.0.2        |
| stimulus        | 2.0.0         | 3.0.1         |
+-----------------+---------------+---------------+
  10 outdated packages found

Related to #19

@simmerz
Copy link

simmerz commented Apr 27, 2022

This is great! Would be amazing to have this in mainline.

@dhh dhh merged commit 871c0b9 into rails:main May 22, 2022
@dhh
Copy link
Member

dhh commented May 22, 2022

This is great! A nice follow up would just be to validate that the URL is as expected, perhaps, and then error out in a predictable way with an explanation. Then perhaps if you just follow along on what changes they may make if any ever, we can keep in sync.

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.

3 participants