chore(deps): resolve browserslist, mysql2 and sanitize-html vulnerabilities - #490
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
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. 📝 WalkthroughWalkthroughThe generated SQL language server dependency manifest now requires Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to 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: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation 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 DocsExplanation 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
Comment |
335fc7c to
8be7eef
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 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:
|
…-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
8be7eef to
5f5a83b
Compare
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
Clears the four advisories that were failing
npx better-npm-audit auditand theaudit-prodCI job.browserslist — lockfile only
Two advisories, one package, both fixed in 4.28.7. No
package.jsonchange: 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-prodonly because@deepnote/sql-language-serverdeclaresjestunderdependencies.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
.deepnotenotebook 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.tswritesdist/sql-lsp-modules/package.json, installs it in isolation, then deletes its lockfile, sonpm auditnever sees it and its declared floor^3.9.8was itself vulnerable. It was patched only by the accident of npm picking the newest 3.x, which does not hold once anode_modulesexists: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_passwordis 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, andSqlLspConnectionhas 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/apputilspins~2.12.1, so a lockfile re-resolve cannot move it (npm reportsfixAvailable: 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-ipywidgets8bundle thatbuild.ts:624-636copies 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 fiveallowedTagsarrays in the bundle containssvg,animate,animateTransform,setorforeignObject— sanitize-html strips them outright. No CI gate covers it and no action is needed.Verification
better-npm-audit auditand--production→ both exit 0 (prod was exit 1 onmain); only the.nsprc-acceptedellipticlow remainsnpm cileavespackage-lock.jsonbyte-identical → lock-drift job passestypecheck,lint,formatclean;npm test→ 2784 passing, 0 failingdataframeRenderer.jsbyte-identical across the browserslist + caniuse-lite bump, so autoprefixer output did not moveFlagged, not addressed here
SqlLspConnectioncannot express TLS (sqlLspConnectionUtils.ts:9-19has nosslfield) — every LSP database connection is unencrypted. This is what makes the mysql2 advisory sharp rather than theoretical.@deepnote/sql-language-server@3.0.0declaresjestunderdependencies, dragging a test framework into the production tree. Belongs upstream.sql-lsp-modulestree ships five floating caret ranges with no lockfile. Onlymysql2is floored here.🤖 Generated with Claude Code
https://claude.ai/code/session_01N3ehgqMdG8GesUkbb83nVp