From bd3bc782ab6c33fe7bbdb9eb2802e219b45c264f Mon Sep 17 00:00:00 2001 From: wenyutang-ms Date: Thu, 7 May 2026 15:25:14 +0800 Subject: [PATCH 1/3] fix: always prepend separator when appending noConfigScripts to PATH EnvironmentVariableCollection.append() performs a literal string concatenation and does not insert a path separator. The previous heuristic checked process.env.PATH on the extension host and skipped the separator when that PATH already ended with one. However, the PATH used by the integrated terminal can differ from the extension host's PATH, so this check could incorrectly drop the separator and glue the noConfigScripts directory onto the last entry of the user's PATH (e.g. C:\Program Files\jreleaser\c:\Users\...\noConfigScripts). Always prepend the separator. A trailing empty PATH entry (if the user's PATH already ended with one) is harmless on both Windows and POSIX shells. Fixes #1637 --- src/noConfigDebugInit.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/noConfigDebugInit.ts b/src/noConfigDebugInit.ts index b0da4f01..2446a0fc 100644 --- a/src/noConfigDebugInit.ts +++ b/src/noConfigDebugInit.ts @@ -93,12 +93,12 @@ export async function registerNoConfigDebug( const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts'); const pathSeparator = process.platform === 'win32' ? ';' : ':'; - // Check if the current PATH already ends with a path separator to avoid double separators - const currentPath = process.env.PATH || ''; - const needsSeparator = currentPath.length > 0 && !currentPath.endsWith(pathSeparator); - const pathValueToAppend = needsSeparator ? `${pathSeparator}${noConfigScriptsDir}` : noConfigScriptsDir; - - collection.append('PATH', pathValueToAppend); + // EnvironmentVariableCollection.append() does literal string concatenation and does + // not insert a separator. Always prepend one so we never glue our directory onto the + // last entry of the user's PATH. We cannot rely on process.env.PATH ending with a + // separator, since the terminal's PATH may differ from the extension host's PATH. + // A trailing empty PATH entry (when PATH already ends with the separator) is harmless. + collection.append('PATH', `${pathSeparator}${noConfigScriptsDir}`); // create file system watcher for the debuggerAdapterEndpointFolder for when the communication port is written const fileSystemWatcher = vscode.workspace.createFileSystemWatcher( From 5fa77fea0c30a71c2e0f21fc4048b8dc55fcc218 Mon Sep 17 00:00:00 2001 From: wenyutang-ms Date: Thu, 7 May 2026 17:00:06 +0800 Subject: [PATCH 2/3] test: add unit tests for noConfigScripts PATH append helper Extract the PATH append-value computation into a vscode-free src/pathUtil.ts module so it can be unit-tested in plain Node mocha. Add test/pathUtil.test.ts covering: - Correct separator on Windows (;), Linux (:), and macOS (:). - The appended value always starts with a path separator (regression guard for #1637). - The scripts directory remains a standalone PATH entry when the user's PATH last entry has no trailing separator (the exact scenario reported in #1637, e.g. C:\Program Files\jreleaser\\). - A trailing separator on the user's PATH only produces a harmless empty entry, never glues another entry onto our scripts dir. - The scripts directory is preserved verbatim at the end of the value. All 9 tests pass under plain mocha; the file is also picked up by the existing Electron-based test runner via the **/**.test.js glob. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/noConfigDebugInit.ts | 10 +--- src/pathUtil.ts | 33 ++++++++++++++ test/pathUtil.test.ts | 98 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 src/pathUtil.ts create mode 100644 test/pathUtil.test.ts diff --git a/src/noConfigDebugInit.ts b/src/noConfigDebugInit.ts index 2446a0fc..2f84ee6b 100644 --- a/src/noConfigDebugInit.ts +++ b/src/noConfigDebugInit.ts @@ -8,6 +8,7 @@ import * as vscode from 'vscode'; import { sendInfo, sendError } from "vscode-extension-telemetry-wrapper"; import { getJavaHome } from "./utility"; +import { buildNoConfigPathAppendValue } from "./pathUtil"; /** * Registers the configuration-less debugging setup for the extension. @@ -91,14 +92,7 @@ export async function registerNoConfigDebug( } const noConfigScriptsDir = path.join(extPath, 'bundled', 'scripts', 'noConfigScripts'); - const pathSeparator = process.platform === 'win32' ? ';' : ':'; - - // EnvironmentVariableCollection.append() does literal string concatenation and does - // not insert a separator. Always prepend one so we never glue our directory onto the - // last entry of the user's PATH. We cannot rely on process.env.PATH ending with a - // separator, since the terminal's PATH may differ from the extension host's PATH. - // A trailing empty PATH entry (when PATH already ends with the separator) is harmless. - collection.append('PATH', `${pathSeparator}${noConfigScriptsDir}`); + collection.append('PATH', buildNoConfigPathAppendValue(noConfigScriptsDir)); // create file system watcher for the debuggerAdapterEndpointFolder for when the communication port is written const fileSystemWatcher = vscode.workspace.createFileSystemWatcher( diff --git a/src/pathUtil.ts b/src/pathUtil.ts new file mode 100644 index 00000000..b6df38ad --- /dev/null +++ b/src/pathUtil.ts @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +/** + * Builds the value to append to PATH for the noConfigScripts directory. + * + * `vscode.EnvironmentVariableCollection.append()` performs literal string + * concatenation and does NOT insert a path separator. We always prepend one so + * we never glue our directory onto the last entry of the user's PATH (e.g. + * `...;C:\Program Files\jreleaser\c:\Users\...\noConfigScripts`). + * + * We cannot rely on `process.env.PATH` ending with a separator: the integrated + * terminal's PATH may differ from the extension host's PATH (it can be modified + * by `terminal.integrated.env.*` or by other extensions' env-var collections). + * + * A leading separator is always safe: if the resolved PATH already ends with + * one, the resulting empty PATH entry is harmless on both Windows and POSIX + * shells. + * + * This module has no `vscode` import so it can be unit-tested in plain Node. + * + * @param scriptsDir absolute path to the noConfigScripts directory + * @param platform the target platform; defaults to `process.platform`. Made + * injectable so unit tests can exercise both Windows and + * POSIX behavior on a single host. + */ +export function buildNoConfigPathAppendValue( + scriptsDir: string, + platform: NodeJS.Platform = process.platform, +): string { + const pathSeparator = platform === 'win32' ? ';' : ':'; + return `${pathSeparator}${scriptsDir}`; +} diff --git a/test/pathUtil.test.ts b/test/pathUtil.test.ts new file mode 100644 index 00000000..d73bde84 --- /dev/null +++ b/test/pathUtil.test.ts @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +import * as assert from "assert"; + +import { buildNoConfigPathAppendValue } from "../src/pathUtil"; + +// Regression tests for issue #1637: the extension was appending its +// noConfigScripts directory to PATH without a separator on some terminal +// PATH configurations, gluing it onto the last entry of the user's PATH. +suite("buildNoConfigPathAppendValue", () => { + + const winDir = "C:\\Users\\me\\.vscode\\extensions\\vscjava.vscode-java-debug-0.59.0\\bundled\\scripts\\noConfigScripts"; + const posixDir = "/home/me/.vscode/extensions/vscjava.vscode-java-debug-0.59.0/bundled/scripts/noConfigScripts"; + + test("uses ';' as separator on Windows", () => { + const result = buildNoConfigPathAppendValue(winDir, "win32"); + assert.strictEqual(result, `;${winDir}`); + }); + + test("uses ':' as separator on Linux", () => { + const result = buildNoConfigPathAppendValue(posixDir, "linux"); + assert.strictEqual(result, `:${posixDir}`); + }); + + test("uses ':' as separator on macOS", () => { + const result = buildNoConfigPathAppendValue(posixDir, "darwin"); + assert.strictEqual(result, `:${posixDir}`); + }); + + test("always starts with a path separator (Windows)", () => { + const result = buildNoConfigPathAppendValue(winDir, "win32"); + assert.ok(result.startsWith(";"), `expected leading ';', got: ${result}`); + }); + + test("always starts with a path separator (POSIX)", () => { + const result = buildNoConfigPathAppendValue(posixDir, "linux"); + assert.ok(result.startsWith(":"), `expected leading ':', got: ${result}`); + }); + + test("never collapses scriptsDir into the previous PATH entry on Windows", () => { + // Simulates the exact scenario from issue #1637: a user PATH whose + // last entry has no trailing separator. After append, the script dir + // must not be glued onto 'jreleaser\'. + const userPath = "C:\\foo;C:\\Program Files\\jreleaser\\"; + const finalPath = userPath + buildNoConfigPathAppendValue(winDir, "win32"); + + const entries = finalPath.split(";"); + assert.ok( + entries.includes("C:\\Program Files\\jreleaser\\"), + `expected 'jreleaser\\' to remain a standalone PATH entry, got entries: ${JSON.stringify(entries)}`, + ); + assert.ok( + entries.includes(winDir), + `expected scripts dir to be a standalone PATH entry, got entries: ${JSON.stringify(entries)}`, + ); + }); + + test("never collapses scriptsDir into the previous PATH entry on POSIX", () => { + const userPath = "/usr/bin:/opt/jreleaser/bin"; + const finalPath = userPath + buildNoConfigPathAppendValue(posixDir, "linux"); + + const entries = finalPath.split(":"); + assert.ok( + entries.includes("/opt/jreleaser/bin"), + `expected '/opt/jreleaser/bin' to remain a standalone PATH entry, got entries: ${JSON.stringify(entries)}`, + ); + assert.ok( + entries.includes(posixDir), + `expected scripts dir to be a standalone PATH entry, got entries: ${JSON.stringify(entries)}`, + ); + }); + + test("yields only an empty (harmless) entry when the user's PATH already ends with a separator", () => { + // If the resolved terminal PATH already ends with ';', append produces + // ';;'. The empty middle entry is ignored by Windows and (in this + // position) effectively a no-op on POSIX shells. + const userPath = "C:\\foo;C:\\bar;"; + const finalPath = userPath + buildNoConfigPathAppendValue(winDir, "win32"); + + const entries = finalPath.split(";"); + // The scripts dir must still be a standalone, valid entry. + assert.ok( + entries.includes(winDir), + `expected scripts dir to remain standalone, got entries: ${JSON.stringify(entries)}`, + ); + // No real entry should be merged with our scripts dir. + assert.ok( + !entries.some((e) => e !== winDir && e.endsWith(winDir)), + `no entry should be glued to scripts dir, got entries: ${JSON.stringify(entries)}`, + ); + }); + + test("scriptsDir appears unchanged at the end of the appended value", () => { + const result = buildNoConfigPathAppendValue(winDir, "win32"); + assert.ok(result.endsWith(winDir), `expected value to end with scriptsDir, got: ${result}`); + }); +}); From 59e72f751bb1187db704a6cdde58fe0f12b60ecc Mon Sep 17 00:00:00 2001 From: wenyutang-ms Date: Thu, 7 May 2026 21:25:57 +0800 Subject: [PATCH 3/3] style: trim verbose comments in pathUtil and tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/pathUtil.ts | 22 +++------------- test/pathUtil.test.ts | 61 ++++++++++--------------------------------- 2 files changed, 18 insertions(+), 65 deletions(-) diff --git a/src/pathUtil.ts b/src/pathUtil.ts index b6df38ad..a9f680b3 100644 --- a/src/pathUtil.ts +++ b/src/pathUtil.ts @@ -4,25 +4,11 @@ /** * Builds the value to append to PATH for the noConfigScripts directory. * - * `vscode.EnvironmentVariableCollection.append()` performs literal string - * concatenation and does NOT insert a path separator. We always prepend one so - * we never glue our directory onto the last entry of the user's PATH (e.g. - * `...;C:\Program Files\jreleaser\c:\Users\...\noConfigScripts`). + * `vscode.EnvironmentVariableCollection.append()` does literal string + * concatenation, so we always prepend a separator to avoid gluing our + * directory onto the last entry of the user's PATH (see issue #1637). * - * We cannot rely on `process.env.PATH` ending with a separator: the integrated - * terminal's PATH may differ from the extension host's PATH (it can be modified - * by `terminal.integrated.env.*` or by other extensions' env-var collections). - * - * A leading separator is always safe: if the resolved PATH already ends with - * one, the resulting empty PATH entry is harmless on both Windows and POSIX - * shells. - * - * This module has no `vscode` import so it can be unit-tested in plain Node. - * - * @param scriptsDir absolute path to the noConfigScripts directory - * @param platform the target platform; defaults to `process.platform`. Made - * injectable so unit tests can exercise both Windows and - * POSIX behavior on a single host. + * @param platform defaults to `process.platform`; injectable for tests. */ export function buildNoConfigPathAppendValue( scriptsDir: string, diff --git a/test/pathUtil.test.ts b/test/pathUtil.test.ts index d73bde84..9f76c8f2 100644 --- a/test/pathUtil.test.ts +++ b/test/pathUtil.test.ts @@ -5,9 +5,7 @@ import * as assert from "assert"; import { buildNoConfigPathAppendValue } from "../src/pathUtil"; -// Regression tests for issue #1637: the extension was appending its -// noConfigScripts directory to PATH without a separator on some terminal -// PATH configurations, gluing it onto the last entry of the user's PATH. +// Regression tests for issue #1637. suite("buildNoConfigPathAppendValue", () => { const winDir = "C:\\Users\\me\\.vscode\\extensions\\vscjava.vscode-java-debug-0.59.0\\bundled\\scripts\\noConfigScripts"; @@ -30,69 +28,38 @@ suite("buildNoConfigPathAppendValue", () => { test("always starts with a path separator (Windows)", () => { const result = buildNoConfigPathAppendValue(winDir, "win32"); - assert.ok(result.startsWith(";"), `expected leading ';', got: ${result}`); + assert.ok(result.startsWith(";")); }); test("always starts with a path separator (POSIX)", () => { const result = buildNoConfigPathAppendValue(posixDir, "linux"); - assert.ok(result.startsWith(":"), `expected leading ':', got: ${result}`); + assert.ok(result.startsWith(":")); }); test("never collapses scriptsDir into the previous PATH entry on Windows", () => { - // Simulates the exact scenario from issue #1637: a user PATH whose - // last entry has no trailing separator. After append, the script dir - // must not be glued onto 'jreleaser\'. + // #1637 scenario: last user PATH entry has no trailing separator. const userPath = "C:\\foo;C:\\Program Files\\jreleaser\\"; - const finalPath = userPath + buildNoConfigPathAppendValue(winDir, "win32"); - - const entries = finalPath.split(";"); - assert.ok( - entries.includes("C:\\Program Files\\jreleaser\\"), - `expected 'jreleaser\\' to remain a standalone PATH entry, got entries: ${JSON.stringify(entries)}`, - ); - assert.ok( - entries.includes(winDir), - `expected scripts dir to be a standalone PATH entry, got entries: ${JSON.stringify(entries)}`, - ); + const entries = (userPath + buildNoConfigPathAppendValue(winDir, "win32")).split(";"); + assert.ok(entries.includes("C:\\Program Files\\jreleaser\\")); + assert.ok(entries.includes(winDir)); }); test("never collapses scriptsDir into the previous PATH entry on POSIX", () => { const userPath = "/usr/bin:/opt/jreleaser/bin"; - const finalPath = userPath + buildNoConfigPathAppendValue(posixDir, "linux"); - - const entries = finalPath.split(":"); - assert.ok( - entries.includes("/opt/jreleaser/bin"), - `expected '/opt/jreleaser/bin' to remain a standalone PATH entry, got entries: ${JSON.stringify(entries)}`, - ); - assert.ok( - entries.includes(posixDir), - `expected scripts dir to be a standalone PATH entry, got entries: ${JSON.stringify(entries)}`, - ); + const entries = (userPath + buildNoConfigPathAppendValue(posixDir, "linux")).split(":"); + assert.ok(entries.includes("/opt/jreleaser/bin")); + assert.ok(entries.includes(posixDir)); }); test("yields only an empty (harmless) entry when the user's PATH already ends with a separator", () => { - // If the resolved terminal PATH already ends with ';', append produces - // ';;'. The empty middle entry is ignored by Windows and (in this - // position) effectively a no-op on POSIX shells. const userPath = "C:\\foo;C:\\bar;"; - const finalPath = userPath + buildNoConfigPathAppendValue(winDir, "win32"); - - const entries = finalPath.split(";"); - // The scripts dir must still be a standalone, valid entry. - assert.ok( - entries.includes(winDir), - `expected scripts dir to remain standalone, got entries: ${JSON.stringify(entries)}`, - ); - // No real entry should be merged with our scripts dir. - assert.ok( - !entries.some((e) => e !== winDir && e.endsWith(winDir)), - `no entry should be glued to scripts dir, got entries: ${JSON.stringify(entries)}`, - ); + const entries = (userPath + buildNoConfigPathAppendValue(winDir, "win32")).split(";"); + assert.ok(entries.includes(winDir)); + assert.ok(!entries.some((e) => e !== winDir && e.endsWith(winDir))); }); test("scriptsDir appears unchanged at the end of the appended value", () => { const result = buildNoConfigPathAppendValue(winDir, "win32"); - assert.ok(result.endsWith(winDir), `expected value to end with scriptsDir, got: ${result}`); + assert.ok(result.endsWith(winDir)); }); });