Bump glob to 10.3.10 (latest).

[pnappa]

The dependency glob was pinned to version 7.1.6, which depended on a package called inflight. inflight is unmaintained, and currently has a memory leak vulnerability, classified high severity.

New major releases of glob do not rely on this vulnerable dependency.

[alangpierce]

@pnappa Thanks for the PR! One issue (failing the build) is that it looks like glob comes with its own types now, so "@types/glob": "^10", is invalid and should be removed.

I definitely want to get this fixed, but want to think a little more on the exact strategy. glob 9 and later (also the change that removes inflight) requires node 16 or higher, whereas Sucrase still officially supports older node versions. The main question on my mind is whether this change needs to be semver-major for Sucrase, of if maybe I can say that those node versions are ancient enough that they're effectively unsupported anyway. Even if it was a Sucrase semver-major release, I'd want to support security fixes for old major versions anyway. I think glob is only used for tsconfig support in the CLI, so it may be possible to only affect those usages. I'll think a little more and get back to this.

[pnappa]

Oops! Silly mistake - amended the commit to remove the types package.

Yeah, the decision of whether a major version is required for semver is somewhat philosophical. I would say yeah, you'd need to bump major because you've specified node version 8 support in the package.json.

I ran: npx ls-engines on main, and yes, all in-branch dependences are node version >= 8:

❯ npx ls-engines
`node_modules` found; loading tree from disk...
┌────────┬─────────────────────────────────────────────────────────────────┐
│ engine │ Currently available latest release of each valid major version: │
├────────┼─────────────────────────────────────────────────────────────────┤
│ node   │ v21.4.0, v20.10.0, v19.9.0, v18.19.0, v17.9.1, v16.20.2,        │
│        │ v15.14.0, v14.21.3, v13.14.0, v12.22.12, v11.15.0, v10.24.1,    │
│        │ v9.11.2, v8.17.0                                                │
└────────┴─────────────────────────────────────────────────────────────────┘

┌──────────────────┬───────────────────────────┐
│ package engines: │ dependency graph engines: │
├──────────────────┼───────────────────────────┤
│ "engines": {     │ "engines": {              │
│   "node": ">= 8" │   "node": ">= 8"          │
│ }                │ }                         │
└──────────────────┴───────────────────────────┘


Your “engines” field exactly matches your dependency graph’s requirements!

┌────────┬─────────────────┬─────────────────┬──────────────────────────┐
│ engine │ current version │ valid (package) │ valid (dependency graph) │
├────────┼─────────────────┼─────────────────┼──────────────────────────┤
│ node   │ v20.3.0         │ yes!            │ yes!                     │
└────────┴─────────────────┴─────────────────┴──────────────────────────┘

And with this patch:

❯ npx ls-engines
`node_modules` found; loading tree from disk...
┌────────┬─────────────────────────────────────────────────────────────────┐
│ engine │ Currently available latest release of each valid major version: │
├────────┼─────────────────────────────────────────────────────────────────┤
│ node   │ v21.4.0, v20.10.0, v19.9.0, v18.19.0, v17.9.1, v16.20.2,        │
│        │ v15.14.0, v14.21.3                                              │
└────────┴─────────────────────────────────────────────────────────────────┘

┌──────────────────┬───────────────────────────┐
│ package engines: │ dependency graph engines: │
├──────────────────┼───────────────────────────┤
│ "engines": {     │ "engines": {              │
│   "node": ">= 8" │   "node": ">= 14.17"      │
│ }                │ }                         │
└──────────────────┴───────────────────────────┘


┌────────┬─────────────────┬─────────────────┬──────────────────────────┐
│ engine │ current version │ valid (package) │ valid (dependency graph) │
├────────┼─────────────────┼─────────────────┼──────────────────────────┤
│ node   │ v20.3.0         │ yes!            │ yes!                     │
└────────┴─────────────────┴─────────────────┴──────────────────────────┘

Your “engines” field allows more node versions than your dependency graph does.

┌───────────────────────────────┬────────────────────┐
│ Conflicting dependencies (16) │ engines.node       │
├───────────────────────────────┼────────────────────┤
│ @isaacs/cliui                 │ >=12               │
├───────────────────────────────┼────────────────────┤
│ @pkgjs/parseargs              │ >=14               │
├───────────────────────────────┼────────────────────┤
│ ansi-regex                    │ >=12               │
├───────────────────────────────┼────────────────────┤
│ ansi-styles                   │ >=12               │
├───────────────────────────────┼────────────────────┤
│ foreground-child              │ >=14               │
├───────────────────────────────┼────────────────────┤
│ glob                          │ >=16 || 14 >=14.17 │
├───────────────────────────────┼────────────────────┤
│ jackspeak                     │ >=14               │
├───────────────────────────────┼────────────────────┤
│ lru-cache                     │ 14 || >=16.14      │
├───────────────────────────────┼────────────────────┤
│ minimatch                     │ >=16 || 14 >=14.17 │
├───────────────────────────────┼────────────────────┤
│ minipass                      │ >=16 || 14 >=14.17 │
├───────────────────────────────┼────────────────────┤
│ path-scurry                   │ >=16 || 14 >=14.17 │
├───────────────────────────────┼────────────────────┤
│ signal-exit                   │ >=14               │
├───────────────────────────────┼────────────────────┤
│ string-width                  │ >=12               │
├───────────────────────────────┼────────────────────┤
│ strip-ansi                    │ >=12               │
├───────────────────────────────┼────────────────────┤
│ wrap-ansi                     │ >=12               │
├───────────────────────────────┼────────────────────┤
│ wrap-ansi-cjs                 │ >=10               │
└───────────────────────────────┴────────────────────┘


If you want to narrow your support, you can run `ls-engines --save`, or manually add the following to your `package.json`:
"engines": {
  "node": ">= 14.17"
}

I didn't even know about the engines field in package.json, I'm surprised they don't warn on installing dependencies that one is violating it - odd!

---

Re: supporting older node.js versions:

I don't think it's necessary to take on additional effort to support very old node versions, even for backporting security patches. There are probably quite a few flaws in the node.js engine itself that are not fixed for unsupported versions: https://nodejs.org/en/blog/vulnerability only mentions that it fixes the versions with security support (see here https://endoflife.date/nodejs). So, updating the engine requirement to be in-line with needs, in addition to bumping the major version & deploying a patch would be productive.

That's my opinion at least - short of forking glob to remove unsupported engine features to keep engine requirement at the same level - it's probably the best path of action.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b9e563f) 88.70% compared to head (305c2eb) 88.70%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #822   +/-   ##
=======================================
  Coverage   88.70%   88.70%           
=======================================
  Files          56       56           
  Lines        6082     6082           
  Branches     1446     1446           
=======================================
  Hits         5395     5395           
  Misses        421      421           
  Partials      266      266           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Benchmark results

Before this PR: 573.8 thousand lines per second
After this PR: 564.8 thousand lines per second

Measured change: 1.57% slower (2.04% slower to 2.03% faster)
Summary: Likely no significant difference

[alangpierce]

Thanks for updating to remove the types package, and thanks again for the PR!

I thought a bit more about the release strategy, and my plan is to release this as a semver-minor release (3.35.0) rather than semver-major, and I'll include a release note explaining caveats and workarounds. It's an awkward choice to make, but I'm weighing the disruption of dropping old node versions with the disruption of a security fix that's only available as semver-major.

Recommendation for projects on Node.js versions before 14.7:

• If the project uses the sucrase CLI, the easiest workaround is to pin Sucrase to 3.34.0 until the project has a chance to upgrade to newer Node.js.
• If the project does not use the sucrase CLI, there are no breaking changes for old Node.js versions. When using package managers like yarn that strictly enforce the engines field, it will be necessary to specify the --ignore-engines flag.

---

To explain the release rationale a bit more, the three options I considered were:

1. Release a new 4.x line, and leave 3.x with the vulnerability.
2. Release a minor or patch release in the 3.x line.
3. Find an alternate non-breaking security fix.

I looked into the details and did some testing to understand the impact more specifically. Some notes and observations:

• The glob package has enough complexity that it didn't seem reasonable to re-implement or fork it. I also took a look at the fast-glob package, but that also lists Node 16 as its minimum version. So I don't see a reasonable path for option 3.
• The only actual use of the glob package is in the Sucrase CLI's partial support of reading tsconfig.json, which was introduced in Support reading information from tsconfig.json #509 . tsconfig's include key lets you specify globs, so any tool that supports tsconfig needs to support globs (plus lots of other complexity). I don't have any metrics, but my impression is that tsconfig support in the CLI is relatively rarely used.
• I said in a previous comment that the requirement was node 16, but glob actually declares it as >=16 || 14 >=14.17. I tested it out on some old docker images and I found that latest glob actually works on Node.js 14.7 (Jul 29, 2020) and later. So I expect the most common case of a breaking change would be people using node 12 and wanting to use the CLI's tsconfig support.
• The impact of the breaking change is an import-time syntax error on the code #fillNegs() { when using the sucrase CLI, since the earlier versions of Node.js don't support private class methods.
• All other usages of Sucrase work fine in earlier Node.js versions, including the Sucrase API (the transform function), the require hook, and other integrations.
• Node.js 16 and earlier are all end-of-life at this point, so anyone on old enough Node.js versions to be affected are already missing security updates in Node itself.
• I'm fairly sure that, by number of users, the vast majority of people installing Sucrase these days are doing so via tailwindcss. Tailwind requires Node.js 16 and also only invokes Sucrase via API (doesn't use the CLI). From looking at dependent projects in the past, I believe this is common; many projects use Sucrase only via API, so a Sucrase semver-major release would be unnecessary for those cases and would require each depending project to upgrade the Sucrase dependency and publish a new release rather than automatically pulling in the security fix.
• I expect that any projects affected by this issue are ones directly using Sucrase as part of their dev environment or build system. It's unfortunate to break these cases, but pinning to an older Sucrase version is comparatively easy when it's a direct dependency rather than indirect.
• I took a look at the behavior of engines, and it looks like for npm, violations are shown as a warning, while for yarn, it's strictly enforced, with an --ignore-engines escape hatch.
• I took a quick look to try to understand how serious the inflight vulnerability is in this context. It appears that the glob function essentially uses inflight for any system calls it runs, which looks like it can be an unbounded number for complex globs, so it seems possible that a malicious tsconfig.json or an unexpectedly large filesystem might cause resource exhaustion noted in the security report. Still, it seems very rare to use an untrusted tsconfig or file system, so the security issue seems unlikely to be an issue in practice.
• From a quick look, I wasn't able to find a lot of precedent for breaking changes released semver-minor to address security issues, though I know I've seen web services reserve the right to immediately release API breaks in response to security vulnerabilities, so this seems similar to me.

Weighing the options, I think the impact of the breaking change would be very small, and would be outweighed by the impact of the Sucrase 3.x line having an open vulnerability, and the impact of all libraries needing to explicitly upgrade.

Some related thoughts on my longer-term thinking for the project:

• At the root of this issue is the fact that Sucrase is trying to be a batteries-included tool and a minimal library at the same time. tailwindcss and others only need the minimal library, but are forced to pull in the CLI and its dependencies. I think the right approach is to split it out into at least two packages, and my plan has been to spin off a @sucrase/core minimal library with few or no dependencies and no integrations. That would significantly reduce the impact of security vulnerabilities like these.
• Like other projects (Babel comes to mind), I've been very hesitant about semver-major releases, and I've been "saving up" some breaking changes for several years. I think there are different schools of thought here, but I do think that breaking changes shouldn't be made lightly.
• Regarding tsconfig support, I think it's a losing battle to try to re-implement tsconfig in all of its complexity. I believe ts-node supports tsconfig by calling into the typescript package directly, so that's probably the ideal approach. Sucrase's ts-node-plugin already bridges between TypeScript's internal representation and Sucrase options, so it should be possible to use that in some form, either in the CLI, as an additional feature in ts-node, or in some other tool.

[pnappa]

Excellent, thank you for the detailed report and feedback.

[alangpierce]

Thanks again! Just released as v3.35.0.