fix(deps): replace unmaintained decompress with targeted archive extraction

[Tatsat (Tats) Mishra 🐉 (Tatsinnit)]

Why

decompress@4.2.1 carries three critical advisories with no fix available. It was last published in 2017 and is unmaintained:

Advisory	Issue
GHSA-mp2f-45pm-3cg9	Archive extraction can create files and links outside the target directory
GHSA-h39j-r5qq-r9mm	Arbitrary file write via archive extraction (Zip Slip)
GHSA-jwp9-9v96-94mx	Arbitrary hardlink creation during archive extraction

npm audit reports these as critical / No fix available, so no version bump resolves them — the dependency has to go.

It is used in exactly one place, binaryDownloadHelper.ts, to unpack the kubelogin and kubectl-gadget release archives.

What changed

Format	Before	After
.zip	decompress	yauzl — the reader behind extract-zip, @vscode/vsce and Electron
.tar.gz	decompress	tar — npm's own implementation

This PR adds zero packages to the dependency tree and removes 51. Both tar@7.5.21 and yauzl@3.4.0 were already resolving transitively (via containerization-assist-mcp and @vscode/vsce respectively) at exactly these versions, so they simply become direct dependencies and dedupe. npm ci produces the same tree you're already running.

yauzl ships no types, and rather than pull in @types/yauzl this adds src/types/yauzl.d.ts — a ~50-line ambient declaration covering only the API actually used. Verified to type-check rather than degrade to any.

The vulnerability class is removed, not mitigated

This is the part worth reviewing. The old code extracted the entire archive to disk, then moved the one file it wanted:

await decompress(downloadFilePath, downloadFolder);          // writes every path in the archive
const unzipped = path.join(downloadFolder, spec.pathToBinaryInArchive);
await moveFile(unzipped, binaryFilePath);

Every path inside the archive got written, which is exactly what makes Zip Slip and hardlink attacks work. The new code extracts only the requested entry, streamed directly to a path we compute ourselves:

await extractBinaryFromArchive(downloadFilePath, spec.pathToBinaryInArchive, partialFilePath);

A malicious archive has no path it can influence, so the bug class is structurally unreachable rather than patched.

Two bugs the rewrite would otherwise have introduced

Windows path separators. Archive entries are always /-separated, but pathToBinaryInArchive is built with path.join, which yields bin\windows_amd64\kubelogin.exe on Windows. The old code sidestepped this by extracting to real filesystem paths; comparing entry names directly would have broken Windows silently. toArchiveEntryPath() normalises before comparing.

Atomicity. Writing straight to binaryFilePath would let a failed extraction leave a truncated binary behind — and getToolBinaryPath() opens with an fs.existsSync(binaryFilePath) cache check, so the next run would hand back the corrupt file. Extraction now goes to a .partial file and renames only on success, with cleanup on failure.

Call sites unchanged

Archive format is inferred from the file extension, so kubeloginDownload.ts and kubectlGadgetDownload.ts are untouched. An unrecognised extension fails with a clear error.

Making it explicit (archiveFormat: "zip" | "tar.gz" on ArchiveDownloadSpec) would move that to compile time and is probably worth doing if more tools get added — it just wasn't worth widening this change. Happy to add it here if preferred.

Verification

Check	Result
tsc -p ./ --noEmit	exit 0
eslint	exit 0
prettier --check	clean
webpack --mode production	exit 0
npm ls decompress	(empty)
npm audit	3 critical → 0 critical
Packages added / removed	0 / 51

Beyond static checks, I exercised the real production getToolBinaryPath() with vscode stubbed, downloading actual GitHub releases at the versions this extension pins (kubelogin v0.2.19, kubectl-gadget v0.53.2):

PASS  kubelogin .zip   (nested entry bin/darwin_arm64/kubelogin)  sha=match mode=755 cleaned=true
PASS  gadget .tar.gz   (root entry kubectl-gadget)                sha=match mode=755 cleaned=true
PASS  missing entry rejected

Both digests are byte-identical to reference extraction via unzip -p and tar xzO, and both binaries execute — kubelogin --version reports v0.2.19, matching the configured release tag. Cache-hit path, archive cleanup, .partial cleanup, missing-entry and unsupported-extension cases all verified.

On the lockfile diff

package-lock.json loses ~580 lines. That's a genuine reduction, not churn — decompress pulled in 51 packages, many long-deprecated:

decompress-tar, decompress-tarbz2, decompress-targz, decompress-unzip, seek-bzip, unbzip2-stream, tar-stream, buffer-alloc, buffer-fill, pinkie, pinkie-promise, is-natural-number, strip-dirs, fd-slicer, plus nested duplicate copies of readable-stream, safe-buffer, string_decoder, isarray and an old yauzl@2.10.0.

The package.json diff itself is six lines. Note npm install also wanted to reformat the unrelated chatSkills array (~110 lines); since .prettierignore excludes **/*.json nothing enforces either style, so I reverted that and hand-applied only the dependency changes to keep this reviewable.

Out of scope

npm audit still reports findings against undici and hono, both pre-existing and unrelated. Separately, hono@4.13.3 (pulled in by #2380) is currently unavailable on the Microsoft package feed proxy — its latest is 4.13.2 — which breaks npm ci locally for anyone on that registry. Not caused by this PR, but worth a look.

[Copilot]

Pull request overview

This PR removes the unmaintained decompress dependency (with critical “no fix available” advisories) by rewriting binary archive extraction to use targeted, single-entry extraction via yauzl (zip) and tar (tar.gz/tgz), reducing the attack surface (no full-archive extraction to disk) and shrinking the dependency tree.

Changes:

• Replace full-archive extraction with streaming extraction of only the requested entry for .zip and .tar.gz/.tgz.
• Add a minimal local ambient type declaration for yauzl to avoid adding @types/yauzl.
• Update dependencies/lockfile: remove decompress (+types) and add direct deps on tar and yauzl.

Reviewed changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/commands/utils/helper/binaryDownloadHelper.ts	Replaces decompress with single-entry extraction for zip/tar archives and adds .partial + rename flow for atomicity.
src/types/yauzl.d.ts	Adds minimal ambient typings for the subset of yauzl used by the helper.
package.json	Removes decompress/@types/decompress; adds tar and yauzl as direct dependencies.
package-lock.json	Reflects dependency tree reduction from removing decompress and promoting tar/yauzl to direct dependencies.

Suppressed comments (2)

src/commands/utils/helper/binaryDownloadHelper.ts:88

• On pipeline(...) rejection, zipFile.close() is never called (the failure handler is reject only). This can leak handles and keep the downloaded archive locked. Ensure the zip is closed in both success and failure cases (e.g., pipeline(...).finally(() => zipFile.close())).

                    pipeline(readStream, fs.createWriteStream(destPath)).then(() => {
                        zipFile.close();
                        resolve();
                    }, reject);

src/commands/utils/helper/binaryDownloadHelper.ts:96

• When the zip ends without finding the entry, the code rejects but does not close the ZipFile. Close the zip before rejecting here to avoid leaking handles / leaving the archive locked.

            zipFile.on("end", () => {
                if (!found) {
                    reject(new Error(`Archive does not contain an entry named ${wantedEntry}.`));
                }
            });

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Suneha Bose (bosesuneha)]

Suggested change

	fs.rmSync(downloadFilePath, { force: true });
	fs.rmSync(downloadFilePath, { force: true });
	download.clear(downloadFilePath);

[Tatsat (Tats) Mishra 🐉 (Tatsinnit)]

Applied — this is the same finding as your other comment, taking the suggestion as written. download.clear() now sits next to the removal in the finally so the file and its download-once marker always move together. Details and the before/after repro are in the reply on the other thread. Fixed in f7858c2.

[Suneha Bose (bosesuneha)]

the finally deletes the archive on failure too, but download.clear() at line 208 is skipped by the early return in the catch. That leaves download.once marked Completed with the file gone, so a retry skips the download and fails until VS Code restarts. On main the archive survived a failed extraction, so retries still worked.

[Tatsat (Tats) Mishra 🐉 (Tatsinnit)]

Confirmed, and you've got the mechanism exactly right — including that main behaved differently. Reproduced it before fixing:

attempt 1 (bad entry) succeeded: false
archive still on disk after failure: []          <- finally removed it
attempt 2 (good entry) succeeded: false
  error: Failed to extract ... /download/kubelogin-darwin-arm64.zip
RETRY IS PERMANENTLY BROKEN

download.once keeps DOWNLOAD_ONCE_STATUS[dest] = Completed in module state, so the second call took the else branch, returned success without downloading, and then extraction failed on a file the finally had already deleted. Permanent for the life of the window, since that map is in-memory.

And your point about main is right: it called fs.unlinkSync only after a successful decompress, so a failed extraction left the archive in place and a retry could still use it. The finally I added is what broke that.

Fixed as you suggested — clearing the marker alongside the removal so the two always move together:

} finally {
    fs.rmSync(downloadFilePath, { force: true });
    download.clear(downloadFilePath);
}

Same repro now:

attempt 1 (bad entry) succeeded: false
attempt 2 (good entry) succeeded: true
  sha256: MATCH
RETRY RECOVERS

Thanks — this was the most consequential of the four and I'd missed it entirely. Fixed in f7858c2.

[Suneha Bose (bosesuneha)]

lgtm