From 514b1eaf347aaeb145b0d7bad0db953ce0b22172 Mon Sep 17 00:00:00 2001 From: kkdev92 Date: Thu, 13 Aug 2026 00:56:10 +0900 Subject: [PATCH] fix: spawn the archiver without a shell in verify-vsix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flags `js/indirect-command-line-injection` here: `process.env.SystemRoot` is interpolated into a string handed to `execSync`. **It is not exploitable and this commit should not claim otherwise.** The value is already quoted, and `SystemRoot` is a trusted value — anyone able to set it can run anything they like without this script's help. Nor does the script ship; it runs locally and in CI to check a packaged VSIX. It is worth changing anyway for one reason that has nothing to do with CodeQL: `vscode-ext-kit`'s equivalent script already uses `execFileSync`, so this is the odd one out. Passing argv removes the quoting question rather than answering it, and removes the shell that could get it wrong. The archive argument stays relative and `cwd` stays as it was — that pairing is deliberate, because bsdtar reads an absolute `C:\…` as a remote host. Verified by running the lane that actually exercises it: package, then `verify:vsix` end to end. Co-Authored-By: Claude Opus 5 --- scripts/verify-vsix.mjs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/scripts/verify-vsix.mjs b/scripts/verify-vsix.mjs index 435eec8..3e0b22a 100644 --- a/scripts/verify-vsix.mjs +++ b/scripts/verify-vsix.mjs @@ -18,7 +18,7 @@ * Usage: node scripts/verify-vsix.mjs [path-to.vsix] */ -import { execSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process'; import { copyFileSync, existsSync, mkdirSync, readdirSync, readFileSync, rmSync } from 'node:fs'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -98,11 +98,18 @@ function extract(vsixPath) { // POSIX use unzip. Relative paths avoid bsdtar misparsing `C:\…` as a remote. const localCopy = join(extractDir, 'package.vsix.zip'); copyFileSync(vsixPath, localCopy); - const command = + // Spawned with an argv array rather than a shell string, so the archiver's path + // is an argument instead of a word to be parsed. `SystemRoot` is a trusted value + // — anyone who can set it can already run anything — but as argv there is no + // quoting to get right and no shell to get it wrong. + const [command, args] = process.platform === 'win32' - ? `"${join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'tar.exe')}" -xf package.vsix.zip` - : 'unzip -q package.vsix.zip'; - execSync(command, { cwd: extractDir, stdio: 'inherit' }); + ? [ + join(process.env.SystemRoot ?? 'C:\\Windows', 'System32', 'tar.exe'), + ['-xf', 'package.vsix.zip'], + ] + : ['unzip', ['-q', 'package.vsix.zip']]; + execFileSync(command, args, { cwd: extractDir, stdio: 'inherit' }); rmSync(localCopy); }