chore(deps): Resolve dependency vulnerabilities (2026-07-27) by tkislan · Pull Request #456 · deepnote/vscode-deepnote

tkislan · 2026-07-27T17:58:08Z

Remediates all 19 non-ignored npm audit advisories currently failing npx better-npm-audit audit, across 9 packages.

Each pin targets the lowest version that clears the advisory while staying inside every consumer's declared range, so no consumer is forced across a major.

Decisions

Package	Sev	Shape	Change
`brace-expansion`	high ×4	range-selector overrides + `.nsprc`	1.1.13→1.1.16, 2.0.3→2.1.2, new 5.0.8 selector
`fast-uri`	high ×2	override, caret→exact	`^3.1.2` → `3.1.4`
`js-yaml`	high ×2	direct dep + new `@3` override	`^4.2.0`→`^4.3.0`, `js-yaml@3`→3.15.0
`postcss`	high	devDep + override, lockstep	both `^8.5.14` → `^8.5.18`
`shell-quote`	high	selector widened	`<1.8.4: 1.8.4` → `<1.9.0: 1.9.0`
`tar`	critical +4	override	`7.5.16` → `7.5.21`
`immutable`	high ×2	new override	`4.3.9`
`dompurify`	low	override	`3.4.11` → `3.4.12`
`body-parser`	low	new override	`2.3.0`

Why each shape

Direct-dep bump only where the package is already a direct dependency — js-yaml (4.x) and postcss. npm rejects a top-level override whose spec string differs from a direct dependency's, so these cannot be overrides.
postcss must change in both places, in lockstep. It is declared in devDependencies and overrides. npm's rule (arborist/lib/node.js assertRootOverrides) is an exact string comparison, not semver compatibility — editing one and not the other is a hard EOVERRIDE and npm install fails outright. It cannot use a range-selector key either, since any selector intersecting ^8.5.x still matches the root edge.
js-yaml@3 selector does not collide with the direct dep. semver.intersects('^4.3.0', '3') is false, so the rule never attaches to the root edge. Verified by a successful install; the in-repo glob-parent@<5.1.2 override alongside a direct glob-parent: ^6.0.2 is the same shape.
Caret → exact pin for fast-uri. The ^3.1.2 caret was leaving 3.1.2 in the lockfile — the identical failure mode aef728e fixed for dompurify. Security overrides in this file are exact (tar, dompurify, flatted, yaml, mermaid).
postcss stays a caret (^8.5.18) deliberately: 8.5.14 does not satisfy ^8.5.18, so npm is forced to re-resolve, and the floor sits above the entire vulnerable range.
tar is 7.5.21, not 7.5.22. npm reported fixAvailable: false because it cannot route a fix through node-gyp's and cacache's ^6 ranges — but the existing pin already forces those to 7.x, so this adds no new compatibility risk. The binding constraint is GHSA-r292-9mhp-454m (<=7.5.20).
body-parser is an override, not an express bump. express@5.2.1 is already latest and declares body-parser: ^2.2.1, which admits 2.3.0.

`.nsprc` changes

Added — GHSA-mh99-v99m-4gvg (brace-expansion, high, OOM crash). Range is <=5.0.7 with no lower bound, so only 5.0.8 escapes it and the EXPANSION_MAX_LENGTH fix was never backported — 1.1.16 and 2.1.2 are the latest 1.x/2.x releases and both remain affected. Forcing those consumers to 5.x is a breaking change, not just a semver violation: 1.x/2.x export a callable default (module.exports = expandTop) while 5.x exports only a named expand, so minimatch@3/5 (require(...)(...)) and minimatch@8/9 (__importDefault(...).default(...)) throw at runtime. minimatch itself tried brace-expansion@^5 in 9.0.6–9.0.8 and reverted in 9.0.9.

Worth reviewer attention: this is the first prod-reachable .nsprc entry — the existing ones are all dev-only. Four production paths stay on 1.1.16/2.1.2. The load-bearing claim is that glob patterns are never built from untrusted input: the shipped call site is FileSystem.globFiles() in src/platform/common/platform/fileSystem.node.ts, which passes extension-authored patterns. Please sanity-check that.

Removed — GHSA-h67p-54hq-rp68 (js-yaml). Its premise is now false. The note claimed "no 3.x backport exists (latest 3.x is 3.14.2, still affected)", but js-yaml 3.15.0 has since been published as the v3-legacy backport, and both consumers (tslint@6.1.3, @istanbuljs/load-nyc-config@1.1.0) declare ^3.13.1. A tarball diff of 3.14.2 → 3.15.0 shows only lib/js-yaml/loader.js changed, with the export block byte-identical and safeLoad/safeDump intact. The new js-yaml@3 override fixes those copies outright.

Gotcha worth knowing for future bumps

npm validates an existing lockfile against raw dependency specs, not overridden ones. Widening a range-selector override is therefore silently ignored when the locked version still satisfies the consumer's own range — npm install reports "up to date" and nothing moves. This bit brace-expansion here (all three lines stayed put across two install passes, and npm update didn't help either); the stale entries had to be dropped from package-lock.json to force re-resolution. Reproduced in isolation to confirm.

Verification

npx better-npm-audit audit — passes
npx better-npm-audit audit --production — passes
tsc --noEmit — passes
npm run test:unittests — 2491 passing, 0 failing
npm run esbuild-all — succeeds (exercises the bumped postcss build path)

Second commit: vulnerable copies shipping in `dist/sql-lsp-modules`

Investigating tar surfaced a packaging hole that overrides could not reach. buildSqlLanguageServer (build/esbuild/build.ts) writes a synthetic package.json into dist/sql-lsp-modules, runs npm install --omit=dev --ignore-scripts there, and deletes the lockfile. That install inherits neither the repo's overrides nor its lockfile, so security pins silently did not apply. Those copies ship, because .vscodeignore's node_modules/** is root-anchored and does not match dist/sql-lsp-modules/node_modules/** — and npm audit cannot see any of it.

Two packages were affected:

Package	Root pin	Actually shipped	Advisory
`ssh2`	`1.17.0`	0.8.9	GHSA-652h-xwhf-q4h6 — high, OS command injection (`<1.4.0`)
`tar`	`7.5.21`	6.2.1	covered by all five current tar advisories

ssh2 is the significant one — it executes. The bundle marks ssh2 and node-ssh-forward external and loads them from dist, so the repo-level pin never protected the shipped SSH tunneling path used by the database integrations. tar is dead weight by comparison: no file in the shipped sqlite3 copy contains require('tar'), and --ignore-scripts means the build-from-source path never runs.

The fix propagates the pins from the root package.json rather than hardcoding them, so a future bump of either override cannot drift out of sync with this build step. A missing or non-string entry fails the build loudly instead of silently shipping an unpinned version.

+const sqlLspOverridesToPropagate = ['ssh2', 'tar'];

     const packageJson = {
         name: 'sql-lsp-deps',
         ...
-        }
+        },
+        overrides
     };

Verified after a clean rebuild — dist/sql-lsp-modules now resolves ssh2@1.17.0 and tar@7.5.21.

Reviewer note — the one thing I could not verify. Forcing ssh2 to 1.17.0 moves node-ssh-forward (which declares ^0.8.9) across a major. Its ssh2 usage is a narrow, stable slice — new Client(), connect({privateKey, agentForward}), forwardOut() — and I confirmed the module loads against 1.17.0 with all three present. The root tree has also already forced this exact combination for some time. But an end-to-end SSH-tunneled database connection was not exercised; that needs a real SSH host. Worth a manual smoke test before release.

Note on the open bot PRs

Renovate/Dependabot have open branches covering 7 of these (tar→7.5.21, dompurify→3.4.12, js-yaml→4.3.0, glob→^10, body-parser→2.3.0, fast-uri→3.1.4, immutable→4.3.9). Per the repo's established practice (aef728e, 8efe0ed, 0ac38a0) this consolidates them into one PR; the bot PRs are intentionally left open to close or conflict out on their own.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YFmRiqkZebrHeTBTHdwV3z

Summary by CodeRabbit

Security
- Updated allowlist entries to address DoS risks in configuration parsing and pattern/brace expansion.
- Refreshed relevant dependency versions affecting stylesheet processing and archive handling.
Build / Packaging
- Ensured packaged language-server installations use the same pinned dependency overrides as the main project, improving consistency and stability.
Bug Fixes
- Improved overall resilience against problematic inputs and related runtime edge cases.

Remediate all 19 non-ignored npm audit advisories reported by better-npm-audit. Fixable advisories are pinned to patched versions (overrides / direct-dep bumps, matching repo convention); the one advisory with no backport is documented as accepted risk in .nsprc. Each pin targets the *lowest* version that clears the advisory while staying inside every consumer's declared range, so no consumer is forced across a major. Pinned to patched versions: - brace-expansion [GHSA-3jxr-9vmj-r5cp, high]: selectors widened per major line — <1.1.13 -> <1.1.16 (1.1.16), >=2.0.0 <2.0.3 -> >=2.0.0 <2.1.2 (2.1.2), plus a new >=5.0.0 <5.0.8 -> 5.0.8. The three selectors are pairwise disjoint and cover every installed copy. - fast-uri [GHSA-v2hh-gcrm-f6hx + GHSA-4c8g-83qw-93j6, high]: override ^3.1.2 -> pinned 3.1.4. Switched caret to an exact pin: the caret was leaving 3.1.2 in the lockfile, the same failure mode aef728e fixed for dompurify. 3.1.4 clears both ranges (<=3.1.3 and <3.1.3) and stays within ajv's ^3.0.1. - js-yaml [GHSA-52cp-r559-cp3m, high]: direct dep ^4.2.0 -> ^4.3.0 for the 4.x copy, plus a new `js-yaml@3` -> 3.15.0 override for the 3.x copies. The @3 selector does not intersect the direct dep's ^4.3.0, so it does not trigger EOVERRIDE (verified by install). - postcss [GHSA-r28c-9q8g-f849, high]: devDependency AND override both ^8.5.14 -> ^8.5.18, changed in lockstep. npm rejects an override whose spec string differs from a direct dependency's, so a one-sided edit is a hard EOVERRIDE failure. - shell-quote [GHSA-395f-4hp3-45gv, high]: selector widened <1.8.4 -> <1.9.0 (1.9.0), since the previously pinned 1.8.4 is itself now vulnerable — same shape as the undici widening in aef728e. - tar [5 advisories, worst critical]: override 7.5.16 -> 7.5.21. The binding constraint is GHSA-r292-9mhp-454m (<=7.5.20). npm reported fixAvailable:false because it cannot route a fix through node-gyp's and cacache's ^6 ranges, but the existing pin already forces those to 7.x, so 7.5.21 adds no new compatibility risk. - immutable [GHSA-v56q-mh7h-f735 + GHSA-xvcm-6775-5m9r, high]: new override 4.3.9. Patch-level bump inside @nteract/types' and @nteract/commutable's declared ^4.0.0-rc.12. - dompurify [GHSA-c2j3-45gr-mqc4, low]: override 3.4.11 -> 3.4.12, within mermaid's ^3.3.1. - body-parser [GHSA-v422-hmwv-36x6, low]: new override 2.3.0. A direct express bump is not available — express@5.2.1 is already latest and declares body-parser ^2.2.1, which admits 2.3.0. Accepted-risk exception added to .nsprc: - brace-expansion [GHSA-mh99-v99m-4gvg, high]: the range is <=5.0.7 with no lower bound, so only 5.0.8 escapes it and the EXPANSION_MAX_LENGTH fix was never backported — 1.1.16 and 2.1.2 are the latest 1.x/2.x releases and both remain affected. Forcing those consumers to 5.x is a breaking change, not just a semver violation: 1.x/2.x export a callable default while 5.x exports only a named `expand`, so minimatch@3/5/8/9 throw at runtime (minimatch itself tried ^5 in 9.0.6-9.0.8 and reverted in 9.0.9). Glob patterns in this extension are never built from untrusted input. Removed from .nsprc: - js-yaml [GHSA-h67p-54hq-rp68]: obsolete. The note claimed "no 3.x backport exists (latest 3.x is 3.14.2)", but js-yaml 3.15.0 has since been published as the v3-legacy backport and both consumers declare ^3.13.1. The new `js-yaml@3` -> 3.15.0 override fixes those copies outright, so the exception is no longer warranted. Note on applying override changes: npm validates an existing lockfile against raw dependency specs, not overridden ones, so widening a range-selector override is silently ignored when the locked version still satisfies the consumer's own range. The stale brace-expansion entries had to be dropped from package-lock.json to force re-resolution. Verified afterwards that all three lines moved. Verified: `npx better-npm-audit audit` and `--production` both pass; `tsc --noEmit` passes; 2491 unit tests pass; `npm run esbuild-all` succeeds (exercises the bumped postcss build path). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFmRiqkZebrHeTBTHdwV3z

coderabbitai · 2026-07-27T17:58:31Z

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1d641ef0-6abe-4322-a3a7-28d5c70ecbcf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

Updates js-yaml, postcss, tar, and several transitive dependency overrides in package.json. Replaces the .nsprc exception for the js-yaml advisory with an accepted-risk entry for the brace-expansion CVE. The SQL language server build now propagates selected root overrides into its generated package metadata.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BuildScript
  participant RootPackageJson
  participant GeneratedSqlLspPackage
  BuildScript->>RootPackageJson: Read selected overrides
  RootPackageJson-->>BuildScript: Return ssh2 and tar versions
  BuildScript->>GeneratedSqlLspPackage: Write validated overrides

Possibly related PRs

deepnote/vscode-deepnote#374: Updates the brace-expansion override and related vulnerability handling.
deepnote/vscode-deepnote#423: Updates the overlapping .nsprc and js-yaml advisory entries.
deepnote/vscode-deepnote#450: Updates the same tar override.

Suggested labels: renovate

Suggested reviewers: dinohamzic, m1so

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Updates Docs	⚠️ Warning	Only build/esbuild/build.ts changed; no docs files were updated in this PR, and the external docs repos aren’t visible here.	Please update the OSS docs and the landing-page roadmap in the private repo, or confirm the feature needs no documentation change.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: dependency updates and override changes to remediate vulnerabilities.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

codecov · 2026-07-27T18:01:37Z

Codecov Report

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

Additional details and impacted files

@@     Coverage Diff     @@
##   main   #456   +/-   ##
===========================
===========================

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

`buildSqlLanguageServer` writes a synthetic package.json into dist/sql-lsp-modules and runs `npm install --omit=dev --ignore-scripts` there, then deletes the lockfile. That install inherits neither the repo's `overrides` nor its lockfile, so transitive dependencies pinned for security at the repo level silently resolved to vulnerable versions inside the shipped VSIX. `.vscodeignore`'s `node_modules/**` is root-anchored and does not match dist/sql-lsp-modules/node_modules/**, so those copies ship. `npm audit` cannot see any of it. Two packages were affected: - ssh2: the root pins 1.17.0, but the isolated install resolved node-ssh-forward's own `^0.8.9` to ssh2@0.8.9 — GHSA-652h-xwhf-q4h6, high, OS command injection (<1.4.0). This is the copy that actually executes: the bundle marks `ssh2` and `node-ssh-forward` external and loads them from dist, so the repo-level pin never protected the shipped SSH tunneling path. - tar: resolved to 6.2.1 via sqlite3's `^6.1.11`, covered by all five current tar advisories. Dead weight rather than live exposure — no file in the shipped sqlite3 copy requires tar, and --ignore-scripts means the build-from-source path never runs — but it still shipped. The pins are sourced from the root package.json rather than hardcoded, so a future bump of either override cannot drift out of sync with this build step; a missing or non-string entry fails the build loudly rather than silently shipping an unpinned version. Verified after a clean rebuild: dist/sql-lsp-modules resolves ssh2@1.17.0 and tar@7.5.21, and node-ssh-forward loads against ssh2 1.x with Client/connect/forwardOut all present — the only API surface it uses (`new Client()`, `connect({privateKey, agentForward})`, `forwardOut()`), all stable across 0.8 -> 1.x. An end-to-end SSH-tunneled database connection was NOT exercised; that needs a real SSH host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFmRiqkZebrHeTBTHdwV3z

coderabbitai

🧹 Nitpick comments (1)

build/esbuild/build.ts (1)
670-670: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use Uri.joinPath() for this new path.

This TypeScript change uses path.join(), contrary to the repository rule. Construct the URI with Uri.joinPath() and pass its filesystem path to fs.readJSON; verify the build runtime supports that URI adapter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@build/esbuild/build.ts` at line 670, Update the package manifest path
construction around rootPackageJson to use Uri.joinPath() with extensionFolder,
then pass the resulting URI’s filesystem path to fs.readJSON; ensure the
selected URI adapter is supported by the build runtime.
Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@build/esbuild/build.ts`:
- Line 670: Update the package manifest path construction around rootPackageJson
to use Uri.joinPath() with extensionFolder, then pass the resulting URI’s
filesystem path to fs.readJSON; ensure the selected URI adapter is supported by
the build runtime.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 240f0166-3129-45ed-bf7b-77dd5907bcce

📥 Commits

Reviewing files that changed from the base of the PR and between 0f7d6f2 and dae1d7d.

📒 Files selected for processing (1)

build/esbuild/build.ts

The Package Lock Drift Check failed: `npm install` rewrote one line of package-lock.json, so the committed lockfile was not what a fresh install produces. Fallout from remediating the brace-expansion advisories. npm ignores a widened range-selector override when the locked version still satisfies the consumer's raw spec, so the stale entries had to be pruned from the lockfile to force re-resolution. Regenerating them wrote the *overridden* spec into the legacy `dependencies` tree that lockfileVersion 2 keeps alongside `packages`: "minimatch": { "version": "3.1.5", "requires": { - "brace-expansion": "1.1.16" + "brace-expansion": "^1.1.7" `requires` records the consumer's declared range, not the resolved version, so it must mirror minimatch@3.1.5's own `^1.1.7` — which is exactly what the authoritative `packages` entry already carried, and what a clean `npm install` normalizes it back to. Audited every other legacy entry referencing brace-expansion; this was the only one affected, matching CI's one-line diff. No resolution change: brace-expansion still resolves to 1.1.16 via the `brace-expansion@<1.1.16` override. Verified: `npm install` followed by `git diff --exit-code HEAD` is now clean, reproducing the CI check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFmRiqkZebrHeTBTHdwV3z

coderabbitai Bot previously approved these changes Jul 27, 2026

View reviewed changes

tkislan dismissed coderabbitai[bot]’s stale review via dae1d7d July 27, 2026 18:21

coderabbitai Bot reviewed Jul 27, 2026

View reviewed changes

coderabbitai Bot previously approved these changes Jul 27, 2026

View reviewed changes

tkislan dismissed coderabbitai[bot]’s stale review via b140079 July 27, 2026 18:28

coderabbitai Bot approved these changes Jul 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(deps): Resolve dependency vulnerabilities (2026-07-27) - #456

chore(deps): Resolve dependency vulnerabilities (2026-07-27)#456
tkislan wants to merge 3 commits into
mainfrom
chore/deps-resolve-vulnerabilities-2026-07-27

tkislan commented Jul 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

Review skipped

Walkthrough

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Jul 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tkislan commented Jul 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Decisions

Why each shape

.nsprc changes

Gotcha worth knowing for future bumps

Verification

Second commit: vulnerable copies shipping in dist/sql-lsp-modules

Note on the open bot PRs

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tkislan commented Jul 27, 2026 •

edited by coderabbitai Bot

Loading

`.nsprc` changes

Second commit: vulnerable copies shipping in `dist/sql-lsp-modules`

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

codecov Bot commented Jul 27, 2026 •

edited

Loading