Skip to content

Replace decompress with yauzl in Ark and Kallichore install scripts - #16256

Open
juliasilge wants to merge 1 commit into
mainfrom
replace-decompress-with-yauzl
Open

juliasilge wants to merge 1 commit into
mainfrom
replace-decompress-with-yauzl

Conversation

@juliasilge

Copy link
Copy Markdown
Member

This PR replaces the decompress dev dependency with yauzl in the install scripts for Ark and Kallichore. It is a follow-up to #16236, and part of setting us up for posit-dev/positron-builds#1269.

Summary

decompress has a critical advisory with no fix. After some other easy-ish fixes, it would be the only critical advisory in positron-r and in positron-supervisor. Only two scripts use it:

  • extensions/positron-r/scripts/install-kernel.ts, which installs Ark
  • extensions/positron-supervisor/scripts/install-kallichore-server.ts, which installs Kallichore

These scripts extract flat zip files that contain one binary and its license files. For this reason, a full archive library is not necessary. The scripts now use yauzl 3.4.0, which positron-duckdb already uses inside of our project. yauzl has no advisories and has one dependency (pend). AT LEAST FOR NOW. 😩

The changes are:

  • Scripts: A small extractZip() helper replaces the call to decompress(). The helper keeps the Unix file mode from the archive, so the binaries stay executable. Zip files made on Windows have no Unix mode, so these files get 0644. yauzl rejects entries with absolute paths or .. segments, which stops a "zip slip" attack.
  • Dependencies: decompress and @types/decompress change to yauzl and @types/yauzl in devDependencies.
  • Lockfiles: The changes remove 31 packages from positron-r and 49 packages from positron-supervisor. The only version change is yauzl from 2.10.0 to 3.4.0. @vscode/vsce in positron-r keeps its own copy of yauzl 2.

The two extensions do not share script code, so each script has its own copy of the helper. If someone feels strongly about not keeping two copies, I am certainly flexible on this.

npm audit results:

Manifest Before After
positron-r 1 critical, 2 high 0 critical, 2 high
positron-supervisor 1 critical, 2 high 0 critical, 2 high

There are no changes to runtime dependencies or to code that we ship.

Not in this PR

  • positron-r still has high advisories for js-yaml and serialize-javascript, from mocha@11.
  • positron-supervisor still has high advisories for axios and form-data. These are runtime dependencies, so a separate PR will need to update them.

Release Notes

New Features

  • N/A

Bug Fixes

  • N/A

Validation Steps

@:critical @:win

This change is only to dev dependencies and build scripts. I did these checks locally on macOS:

  • I removed resources/ark and resources/kallichore. Then I ran npm run install-kernel and npm run install-kallichore. The extracted files are the same as the output of unzip. ark and kcserver are executable and start correctly.
  • I replaced kcserver with a file that is not executable, and I ran the install again. The script replaced the file, and the new binary is executable.
  • I extracted a Windows zip for Kallichore. The files have the mode 0644.
  • I extracted a zip that has a ../ entry. The helper rejected the zip, and it wrote no files outside the target folder.

CI builds and the Windows tests run these scripts on Linux and Windows.

@github-actions

Copy link
Copy Markdown

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:win @:ark @:sessions @:console @:interpreter

Why these tags?
Tag Source
@:critical Always runs (required)
@:win PR description
@:ark Changed files
@:sessions Changed files
@:console Changed files
@:interpreter Changed files

More on automatic tags from changed files.

readme  valid tags

@juliasilge

Copy link
Copy Markdown
Member Author

@juliasilge
juliasilge marked this pull request as ready for review September 26, 2026 02:14
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.

1 participant