Skip to content

chore(deps): resolve browserslist, mysql2 and sanitize-html vulnerabilities - #490

Merged
tkislan merged 2 commits into
mainfrom
chore/deps-vulnerabilities-2026-09-01
Sep 2, 2026
Merged

chore(deps): resolve browserslist, mysql2 and sanitize-html vulnerabilities#490
tkislan merged 2 commits into
mainfrom
chore/deps-vulnerabilities-2026-09-01

Conversation

@tkislan

@tkislan tkislan commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Clears the four advisories that were failing npx better-npm-audit audit and the audit-prod CI job.

Advisory Package Fix Why this shape
GHSA-c83g-rgw3-j3cx browserslist Lockfile bump → 4.28.8 Nothing floors it; all consumer ranges already admit the patch
GHSA-73wf-gq98-2v4g browserslist (same bump) Same package, same range, same patched version
GHSA-3f6p-5ww8-9rcr mysql2 Lockfile bump → 3.24.2 + raise the shipped tree's floor The shipped copy is a separate, unaudited tree
GHSA-g8qq-57p8-ggw5 sanitize-html Override bump 2.17.6 → 2.17.7 The existing override was itself pinning it to the vulnerable version

browserslist — lockfile only

Two advisories, one package, both fixed in 4.28.7. No package.json change: no override key exists and all three consumer ranges (^4.24.0, ^4.24.4, >= 4.21.0) already admit the patch, so the condition that forced overrides in eab374b doesn't hold.

Not reachable — build-time only, present in no esbuild metafile. It gates audit-prod only because @deepnote/sql-language-server declares jest under dependencies.

mysql2 — lockfile bump plus a build-script floor

A rogue or MITM'd server forces an auth-plugin downgrade and the client replies with the plaintext password. On a live credential path: opening a .deepnote notebook connects with a plaintext password and no TLS.

The root tree is a lockfile bump. The copy that actually ships is a different tree — build/esbuild/build.ts writes dist/sql-lsp-modules/package.json, installs it in isolation, then deletes its lockfile, so npm audit never sees it and its declared floor ^3.9.8 was itself vulnerable. It was patched only by the accident of npm picking the newest 3.x, which does not hold once a node_modules exists:

seeded: mysql2@3.15.3 (vulnerable)
  ^3.9.8   -> 3.15.3   still vulnerable
  ^3.22.0  -> 3.24.2   patched
-            mysql2: '^3.9.8',
+            mysql2: '^3.22.0',

Propagating a root override instead was tested, not assumed: a plain pin fails with EOVERRIDE: Override for mysql2@^3.9.8 conflicts with direct dependency (mysql2 is a direct dep of that manifest; the four existing propagated entries are transitive). A ranged key does work and reaches the same 3.24.2, but costs a root override plus the only ranged entry in that list for an identical floor.

Behaviour change: after 3.22.0 a server requesting mysql_clear_password is refused unless the client opts in. That is the point of the fix, but MySQL setups that legitimately rely on cleartext auth (PAM/LDAP-backed) will stop authenticating, and SqlLspConnection has no way to set that flag today.

sanitize-html — override bump

Published after the first three fixes and turned the audit red again. The one advisory here that genuinely needs the override: @jupyterlab/apputils pins ~2.12.1, so a lockfile re-resolve cannot move it (npm reports fixAvailable: false).

Scope is one resolved package, nothing added or removed.

This clears the dependency graph, not shipped code. sanitize-html is in none of our esbuild metafiles; the only copy that ships is inlined in the prebuilt @vscode/jupyter-ipywidgets8 bundle that build.ts:624-636 copies verbatim. Overrides govern npm resolution, not code already bundled upstream, so that copy moves only when upstream republishes.

That copy is not exploitable: the advisory needs <animate>/<set> inside <svg> to survive sanitization, and none of the five allowedTags arrays in the bundle contains svg, animate, animateTransform, set or foreignObject — sanitize-html strips them outright. No CI gate covers it and no action is needed.

Verification

  • better-npm-audit audit and --production → both exit 0 (prod was exit 1 on main); only the .nsprc-accepted elliptic low remains
  • npm ci leaves package-lock.json byte-identical → lock-drift job passes
  • esbuild build, typecheck, lint, format clean; npm test → 2784 passing, 0 failing
  • dataframeRenderer.js byte-identical across the browserslist + caniuse-lite bump, so autoprefixer output did not move

Flagged, not addressed here

  1. SqlLspConnection cannot express TLS (sqlLspConnectionUtils.ts:9-19 has no ssl field) — every LSP database connection is unencrypted. This is what makes the mysql2 advisory sharp rather than theoretical.
  2. @deepnote/sql-language-server@3.0.0 declares jest under dependencies, dragging a test framework into the production tree. Belongs upstream.
  3. The generated sql-lsp-modules tree ships five floating caret ranges with no lockfile. Only mysql2 is floored here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01N3ehgqMdG8GesUkbb83nVp

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: fc71b0ef-3bb5-4d0c-b70a-f0b7cd9bf16c

📥 Commits

Reviewing files that changed from the base of the PR and between 5f5a83b and f8caac1.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • package.json

Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.


📝 Walkthrough

Walkthrough

The generated SQL language server dependency manifest now requires mysql2 version ^3.22.0 instead of ^3.9.8. The sanitize-html dependency override now uses version 2.17.7.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to f8caa

The dependency updates and shipped MySQL version floor are supported by passing verification, and no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: jamesbhobbs, dinohamzic

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Updates Docs ✅ Passed This PR is dependency and security maintenance. It does not implement a feature. The diff contains no documentation or roadmap files, so the feature-documentation check is not applicable. This checkou…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely identifies the dependency vulnerability fixes for browserslist, mysql2, and sanitize-html.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

Full details: Updates Docs

Explanation

This PR is dependency and security maintenance. It does not implement a feature. The diff contains no documentation or roadmap files, so the feature-documentation check is not applicable. This checkout exposes only deepnote/vscode-deepnote; if the mysql2 authentication behavior change requires user-facing documentation, review the deepnote OSS documentation and the private deepnote-internal landing-page roadmap separately.

  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

@tkislan
tkislan force-pushed the chore/deps-vulnerabilities-2026-09-01 branch from 335fc7c to 8be7eef Compare September 1, 2026 20:21
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 37%. Comparing base (c2e8f18) to head (f8caac1).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #490   +/-   ##
=====================================
  Coverage     37%     37%           
=====================================
  Files        828     828           
  Lines      41679   41679           
  Branches    9136    9136           
=====================================
  Hits       15449   15449           
  Misses     24116   24116           
  Partials    2114    2114           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 1, 2026
@tkislan
tkislan marked this pull request as ready for review September 2, 2026 05:16
@tkislan
tkislan requested a review from a team as a code owner September 2, 2026 05:16
Comment thread build/esbuild/build.ts Outdated
…-01)

Clears the three high-severity advisories that were failing
`npx better-npm-audit audit` (and the `audit-prod` CI job). Both packages
reach the tree through `@deepnote/sql-language-server`, but they need
different remediations, and neither needs a root `overrides` entry.

- browserslist 4.26.3 -> 4.28.8 (GHSA-c83g-rgw3-j3cx unbounded cache growth,
  GHSA-73wf-gq98-2v4g normalizeStats crash/prototype write; both fixed in
  4.28.7). Lockfile-only: nothing floors this package. All three consumer
  ranges are open (`^4.24.0`, `^4.24.4`, `>= 4.21.0`) and no `browserslist`
  override key exists, so the condition that forced overrides in eab374b
  ("the existing override key was itself pinning the tree to a now-vulnerable
  version") does not hold. Neither defect is reachable — the repo has no
  `.browserslistrc`, no `browserslist` package.json key and no
  `browserslist-stats.json`, and browserslist appears in none of the esbuild
  metafiles, so it is build-time only and ships in no bundle. It is in the
  production tree, and so gates `audit-prod`, only because
  `@deepnote/sql-language-server` declares `jest` under `dependencies`.

- mysql2 3.15.3 -> 3.24.2 (GHSA-3f6p-5ww8-9rcr, fixed in 3.22.0). A rogue or
  MITM'd server sends an AuthSwitchRequest that pre-3.22.0 clients honour
  unconditionally, leaking the plaintext password; 3.22.0 puts
  mysql_clear_password behind an opt-in flag. This one is on a live
  credential path: opening a .deepnote notebook connects with a plaintext
  password and no TLS.

  The root tree is a lockfile bump for the same reason as browserslist. The
  copy that actually ships is separate: build.ts writes
  dist/sql-lsp-modules/package.json, npm-installs it in isolation and then
  deletes its lock file, so it is invisible to npm audit and its declared
  floor `^3.9.8` was itself vulnerable. It was patched only by accident of
  npm picking the newest 3.x — and that accident does not hold when a
  node_modules already exists: a tree seeded at 3.15.3 stays at 3.15.3 under
  `^3.9.8`, and moves to 3.24.2 under `^3.22.0`. Raising the declared floor
  is the fix. The reasoning lives here rather than in a code comment.

  Propagating a root override was measured against this, not assumed. A plain
  pin — the shape of all four existing entries in sqlLspOverridesToPropagate,
  and what the string lookup at build.ts:689-697 implies — fails outright,
  because mysql2 is a direct dependency of that generated manifest whereas
  those four are transitive there:
      npm error EOVERRIDE
      npm error Override for mysql2@^3.9.8 conflicts with direct dependency
  A ranged key ("mysql2@<3.22.0": "3.22.0") does install, and resolves to the
  same 3.24.2 — but it buys an identical floor at the cost of a root override
  plus the only ranged entry in that list. The one-line floor is smaller.

Verified: `better-npm-audit audit` and `--production` both exit 0 (only the
.nsprc-accepted elliptic entry remains); `npm ci` leaves package-lock.json
byte-identical; esbuild build, tsc, oxlint and prettier all clean; 2784 unit
tests pass. The dataframeRenderer bundle is byte-identical across the
browserslist and caniuse-lite bump, so autoprefixer output is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N3ehgqMdG8GesUkbb83nVp
coderabbitai[bot]
coderabbitai Bot previously approved these changes Sep 2, 2026
GHSA-g8qq-57p8-ggw5 (moderate, CVSS 5.4) landed after the browserslist and
mysql2 fixes and turned `better-npm-audit audit` red again. Stored XSS via an
SVG SMIL URI-list scheme-policy bypass; range `>=1.9.0 <=2.17.6`, fixed in
2.17.7.

Unlike the other two advisories in this branch, this one genuinely needs the
override — it is the case eab374b describes, where the existing override key
was itself pinning the tree to what is now the vulnerable version. A lockfile
re-resolve cannot move it: `@jupyterlab/apputils` pins `~2.12.1` and
`@jupyter-widgets/base-manager` asks for `^2.3`, which is why npm reports
`fixAvailable: false`. Bumping the override value 2.17.6 -> 2.17.7 is the same
shape as the dompurify/mermaid/js-yaml bumps in cd830f4.

Scope of the change is one resolved package: `sanitize-html 2.17.6 -> 2.17.7`,
nothing added or removed.

Worth recording, because the override does not reach it: the copy of
sanitize-html that actually ships is frozen inside the prebuilt
`@vscode/jupyter-ipywidgets8` bundle, which build.ts:624-636 copies verbatim
into dist/renderers/ipywidgets8/ipywidgets.js (byte-identical to
node_modules/@vscode/jupyter-ipywidgets8/dist/ipywidgets.js). `overrides`
governs npm resolution, not code already bundled by a published package, so
that copy moves only when upstream republishes. sanitize-html appears in none
of this repo's own esbuild metafiles.

Verified: `better-npm-audit audit` and `--production` both exit 0; `npm ci`
leaves package-lock.json byte-identical; tsc, prettier clean; 2784 unit tests
pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N3ehgqMdG8GesUkbb83nVp
@tkislan tkislan changed the title chore(deps): resolve browserslist and mysql2 vulnerabilities (2026-09-01) chore(deps): resolve browserslist, mysql2 and sanitize-html vulnerabilities Sep 2, 2026
@tkislan
tkislan requested a review from m1so September 2, 2026 08:10
@tkislan
tkislan merged commit 2481e2b into main Sep 2, 2026
19 checks passed
@tkislan
tkislan deleted the chore/deps-vulnerabilities-2026-09-01 branch September 2, 2026 08:12
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