Skip to content

Patch js-yaml merge-key DoS (SNYK-JS-JSYAML-17900054)#1056

Merged
juliasilge merged 1 commit into
mainfrom
chore/js-yaml-bump
Jul 13, 2026
Merged

Patch js-yaml merge-key DoS (SNYK-JS-JSYAML-17900054)#1056
juliasilge merged 1 commit into
mainfrom
chore/js-yaml-bump

Conversation

@juliasilge

Copy link
Copy Markdown
Collaborator

Re-resolve the transitive js-yaml dependency to patched versions within each major line (4.3.0 for the ^4.1.0 consumers, 3.15.0 for eslint's transitive ^3.13.1), closing the quadratic-complexity merge-key DoS advisory (SNYK-JS-JSYAML-17900054 / CVE-2026-53550) across all dependency paths. The same bump also clears the related prototype-pollution-in-merge advisory (CVE-2025-64718, fixed in 3.14.2 and 4.1.1) on those same paths.

This supersedes the Snyk PRs #1048, #1049, #1050, #1051, and #1054, each of which proposed bumping js-yaml to 4.3.0 in a single package.json (quarto-core, editor-server, lsp, vscode, and vscode-markdownit), and #1053, which proposed bumping eslint from 7 to 9 in quarto-ojs-runtime solely to drag its transitive js-yaml 3.x off the vulnerable version. The lockfile re-resolution fixes every path at once, including the transitive js-yaml 3.x pulled in by eslint, with no package.json changes and no eslint 9 flat-config migration. js-yaml's public API is unchanged across these patch/minor bumps (both the 3.x and 4.x lines keep the same load/dump API and the same argparse/esprima deps), so there is no behavior change.

One dev-only copy of js-yaml 4.1.0 remains, pinned exactly by mocha 9's test runner; it is not reachable by lockfile re-resolution and only ever parses trusted local config, so it is left as-is.

This PR addresses part of #1052 but not all of that one; I think we'll handle that separately.

@posit-snyk-bot

posit-snyk-bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@juliasilge juliasilge requested a review from cscheid July 13, 2026 15:22
@cscheid

cscheid commented Jul 13, 2026

Copy link
Copy Markdown
Member

In order to get good yaml squiggles, we use js-yaml is a bit of a weird way to build the annotated YAML parses that include source location information. I think this is safe, but I would probably test the YAML validation infra inside the extension just to make sure it's still good.

@juliasilge

Copy link
Copy Markdown
Collaborator Author

Thanks, @cscheid! If you trace how the YAML validation actually runs, this bump doesn't touch the path makes these kinds of squiggles:

Screenshot 2026-07-13 at 9 31 58 AM

The squiggles (e.g. "potato" not being a valid format) are produced by the installed quarto CLI, not by this repo. provideYamlDiagnostics just calls quarto.getYamlDiagnostics, which is getLint loaded from the quarto installation's editor/tools/vs-code.mjs (dynamically imported in apps/lsp/src/quarto.ts), and it renders the returned lint items (they come back tagged source: "quarto"). So the validation engine, and whatever js-yaml it bundles, is external to this change.

The annotated-parse code you're probably thinking of lives in packages/annotated-json here, but it's published for the CLI to bundle and isn't imported anywhere in the extension/LSP runtime. It also rides on a hand-patched vendored js-yaml (src/external/js-yaml.js), not the npm dependency, so a lockfile re-resolution wouldn't change it regardless. And Snyk doesn't look at that kind of vendored code at all!

What this repo's js-yaml is actually used for is the extension's own front-matter and resource parsing, e.g. reading the shipped attrs.yml for attribute completions and the front matter/config parsing in core / quarto-core. Those use the standard load/dump API, which is unchanged from 4.1.0 to 4.3.0, so no behavior change there either.

We are running the vscode main suite (diagnostics + yamlProviders) to confirm nothing regressed and it looks good.

@cscheid

cscheid commented Jul 13, 2026

Copy link
Copy Markdown
Member

I forgot that we still use that vendored JS blob for the VS code extension as well! (we have to do it in RStudio but I thought we had brought up that YAML code into the monorepo. Too. Many. Small. Differences! My apologies.) But that's good news here.

@cscheid cscheid 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.

LGTM!

@juliasilge juliasilge merged commit 81eec90 into main Jul 13, 2026
4 checks passed
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.

3 participants