Skip to content

Fix npm audit vulnerabilities - #9

Merged
lyonsil merged 3 commits into
mainfrom
fix/npm-audit
Sep 29, 2026
Merged

lyonsil merged 3 commits into
mainfrom
fix/npm-audit

Conversation

@lyonsil

@lyonsil lyonsil commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Summary

npm audit now reports 0 vulnerabilities. Before this change it reported 30: 1 critical, 21 high, 3 moderate and 5 low.

  • npm audit fix makes semver-compatible lockfile bumps that clear most of the advisories. They include @xmldom/xmldom, picomatch, serialize-javascript, tar-fs, webpack, lodash, js-yaml, ajv and brace-expansion.
  • sqlite3 goes from ^5.1.6 to ^6.0.1. This is a major version bump. 6.0.1 is the only release without the vulnerable node-gyp, make-fetch-happen, cacache and tar chain, which includes the critical tar advisories. The JS API we use is unchanged, and it needs Node >= 20.17.
  • Node is pinned to 22.22.1 with Volta, matching paranext-core. Both workflows now install the pinned version instead of Node 20 (build-english-db.yml) or the runner default (test.yml), and @types/node is bumped to ^22.
  • overrides pins minimatch to ^9.0.7 under @typescript-eslint/typescript-estree@6 only. The v6 parser that eslint-config-erb pulls in pins minimatch@9.0.3 exactly, so npm audit fix cannot update it. The override keeps lint behavior unchanged. Another fix is to drop the unused eslint-config-erb and move to typescript-eslint v8's parser and plugin, but that turns on new lint rules, so it belongs in a separate PR.
  • sqlite3 on older Linux: the sqlite3 6 Linux prebuild is linked against glibc 2.38. On Ubuntu 22.04, loading it fails with GLIBC_2.38' not found, so a postinstall script (scripts/ensure-sqlite3.js) rebuilds sqlite3 from source only when the prebuilt binary fails to load for that reason. It exits immediately everywhere else, including CI on ubuntu-latest (24.04). The README lists the build tools that older distros need.

Verification

All of this was run locally after a clean npm ci, against the current ../marble-lexicon and ../marble-indexes with a pinned --version. The XML and DB comparisons ran on Node 20, before the Volta pin. The other checks were re-run on the pinned Node 22.22.1:

  • On Ubuntu 22.04 (glibc 2.35), a clean npm ci triggers the postinstall rebuild, and sqlite3 loads afterwards.
  • npm test passes (18/18), and npm run lint and npx tsc --noEmit are clean.
  • convert-marble-lexicon for SDBH and SDBG produces XML byte-identical to the output built with the old dependencies.
  • import-lexicon-to-sqlite builds a DB with the same schema and the same content in every table. I compared them after resolving surrogate keys to natural keys (6.4M sense occurrences, 684K sense domains, 635K glosses and so on).
    • A raw key-for-key comparison does not match. That comes from existing behavior, not from this change: the importer inserts each batch of entries concurrently (Promise.all), so EntryKey and SenseKey values change from run to run even with the same dependencies.

🤖 Generated with Claude Code


This change is Reviewable

Apply the non-breaking npm audit fixes, upgrade sqlite3 to 6.0.1 (the only
release free of the vulnerable tar/node-gyp chain), and override minimatch
under @typescript-eslint/typescript-estree, which pins a vulnerable 9.0.3.

sqlite3 6's Linux prebuild needs glibc 2.38+, so the README now says how to
build it from source on older distros.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@tjcouch-sil tjcouch-sil left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also worth looking at, though not part of this PR's changes: the workflows don't use a consistent Node version. build-english-db.yml pins node-version: '20', and Node 20 reached end of life in April 2026. test.yml has no setup-node step, so it uses whatever Node version ubuntu-latest happens to have.

Once there's a Volta pin (see the comment on package.json), it would be best to install that version in the workflows the way paranext-core does, so CI and developers use the same Node:

- name: Read package.json
  id: package_json
  uses: zoexx/github-action-json-file-properties@1.0.6
  with:
    file_path: 'package.json'

- name: Install Volta and toolchain
  uses: volta-cli/action@v4

- name: Install Node and NPM
  uses: actions/setup-node@v4
  with:
    node-version: ${{ fromJson(steps.package_json.outputs.volta).node }}
    cache: npm

Comment thread README.md Outdated
Comment thread package.json
Comment thread package.json Outdated
- Scope the minimatch override to typescript-estree v6 so newer v8
  releases keep the minimatch ^10 they require.
- Pin Node 22.22.1 with Volta (sqlite3 6 needs >= 20.17) and have both
  workflows install the pinned version instead of Node 20 / runner
  default. Bump @types/node to match.
- Add a postinstall script that rebuilds sqlite3 from source only when
  the prebuilt binary fails to load for lack of glibc 2.38, so the fix
  survives reinstalls. Update the README accordingly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lyonsil

lyonsil commented Sep 28, 2026

Copy link
Copy Markdown
Member Author

Re: the workflow Node versions - done in this PR. Both workflows now read the Volta version from package.json and install it the way paranext-core does.

(AI-assisted, with my guidance)

Comment thread .github/workflows/build-english-db.yml Outdated
Comment thread package.json
Comment thread scripts/ensure-sqlite3.js
…e TODO

Read the Volta Node version with setup-node's node-version-file instead of
two third-party actions, and clone the private repos after npm ci so no
third-party or dependency install code runs with the token on disk.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lyonsil
lyonsil merged commit 627e0f5 into main Sep 29, 2026
2 checks passed
@lyonsil
lyonsil deleted the fix/npm-audit branch September 29, 2026 17:59
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.

2 participants