From fb555ae65e9e86322aeb5eab74f5680a1eaa9569 Mon Sep 17 00:00:00 2001 From: netcatty-bot <308658023+netcatty-bot@users.noreply.github.com> Date: Sat, 5 Sep 2026 09:16:11 +0000 Subject: [PATCH] fix(#3280): automated AI fix --- electron/bridges/shellDiscovery.cjs | 132 +++++++++++++++---- electron/bridges/shellDiscovery.test.cjs | 156 +++++++++++++++++++++++ electron/bridges/terminalBridge.cjs | 43 +++---- 3 files changed, 283 insertions(+), 48 deletions(-) create mode 100644 electron/bridges/shellDiscovery.test.cjs diff --git a/electron/bridges/shellDiscovery.cjs b/electron/bridges/shellDiscovery.cjs index 443cd70e2..97fa3fbb2 100644 --- a/electron/bridges/shellDiscovery.cjs +++ b/electron/bridges/shellDiscovery.cjs @@ -88,9 +88,106 @@ function regEnumSubkeys(keyPath) { return []; } +/** + * Absolute path prefix of the per-user Windows App Execution Aliases + * directory (`%LOCALAPPDATA%\Microsoft\WindowsApps`), lower-cased and + * terminated with a path separator — or `null` when it cannot be + * determined (non-Windows platforms, missing LOCALAPPDATA). + * + * @param {NodeJS.ProcessEnv} [env] Environment to read LOCALAPPDATA from. + * @param {string} [platform] Platform override (tests). + * @returns {string|null} + */ +function windowsAppsAliasPrefix(env = process.env, platform = process.platform) { + if (platform !== "win32") return null; + const localAppData = env.LOCALAPPDATA || ""; + if (!localAppData) return null; + // Win32 paths always use backslashes; build the prefix explicitly so the + // check does not depend on the path module of the host OS (tests). + return ( + localAppData.replace(/[\\/]+$/, "") + + "\\Microsoft\\WindowsApps\\" + ).toLowerCase(); +} + +/** + * Check whether a candidate path lives in the per-user Windows App + * Execution Aliases directory (e.g. the MSIX/Store `pwsh.exe` alias). + * + * @param {string} candidate Candidate executable path. + * @param {NodeJS.ProcessEnv} [env] Environment to read LOCALAPPDATA from. + * @param {string} [platform] Platform override (tests). + * @returns {boolean} + */ +function isWindowsAppExecutionAlias(candidate, env = process.env, platform) { + if (!candidate || typeof candidate !== "string") return false; + const prefix = windowsAppsAliasPrefix(env, platform); + if (!prefix) return false; + return ( + candidate.replace(/\//g, "\\").toLowerCase().startsWith(prefix) + ); +} + +/** + * Pick the first launchable executable path from an ordered candidate list + * (e.g. the lines `where.exe ` printed, in PATH order). + * + * Regular files are verified with `fs.existsSync()`. Candidates inside the + * WindowsApps alias directory need special handling: MSIX/Store packages + * expose their executables as AppExecLink reparse points, which Node's + * `fs.statSync()`/`fs.existsSync()` cannot resolve (they fail with EACCES, + * see nodejs/node#36790) even though CreateProcess launches them fine via + * the full alias path. For those, fall back to `fs.lstatSync()`, which + * still sees the reparse point itself. A zero-byte plain file in the same + * directory (e.g. a disabled alias) stats cleanly and is rejected. + * + * Non-alias candidates win over alias candidates so a real install (MSI, + * zip, Chocolatey…) is still preferred when both are on PATH. + * + * @param {string[]} candidates Ordered candidate paths. + * @param {{fs?: typeof import("node:fs"), env?: NodeJS.ProcessEnv, platform?: string}} [deps] Injectable for tests. + * @returns {string|null} First launchable path, or `null`. + */ +function selectExecutableCandidate(candidates, deps = {}) { + const fsMod = deps.fs || fs; + const env = deps.env || process.env; + const platform = deps.platform || process.platform; + let aliasFallback = null; + + for (const candidate of candidates) { + if (!candidate) continue; + + if (isWindowsAppExecutionAlias(candidate, env, platform)) { + if (aliasFallback) continue; + // existsSync() is false for AppExecLink reparse points, so a plain + // existence check would filter the alias out. Verify via statSync: + // success means a plain (non-launchable) stub; failure means a + // reparse point, confirmed with lstatSync. + try { + fsMod.statSync(candidate); + continue; // Plain zero-byte stub — not an AppExecLink. + } catch (_statErr) { + // Expected for AppExecLink reparse points. + } + try { + fsMod.lstatSync(candidate); + } catch (_lstatErr) { + continue; + } + aliasFallback = candidate; + continue; + } + + if (!fsMod.existsSync(candidate)) continue; + return candidate; + } + + return aliasFallback; +} + /** * Locate an executable on the system PATH using `where.exe`. - * Returns the first valid, non-alias path, or `null` if not found. + * Returns the first valid, launchable path, or `null` if not found. * * @param {string} name Executable name, e.g. "pwsh" * @returns {string|null} @@ -98,30 +195,12 @@ function regEnumSubkeys(keyPath) { function findExecutableOnPath(name) { try { const result = execFileSync("where.exe", [name], EXEC_OPTS); - const candidates = result - .split(/\r?\n/) - .map((l) => l.trim()) - .filter(Boolean); - - for (const candidate of candidates) { - if (!fs.existsSync(candidate)) continue; - // Skip Windows App Execution Aliases (WindowsApps zero-byte stubs). - try { - const localAppData = (process.env.LOCALAPPDATA || "").toLowerCase(); - if ( - localAppData && - candidate.toLowerCase().startsWith( - path.join(localAppData, "Microsoft", "WindowsApps").toLowerCase() + - path.sep, - ) - ) { - continue; - } - } catch (_e) { - // Ignore — just use the candidate. - } - return candidate; - } + return selectExecutableCandidate( + result + .split(/\r?\n/) + .map((l) => l.trim()) + .filter(Boolean), + ); } catch (_err) { // Not found on PATH. } @@ -709,5 +788,8 @@ module.exports = { regQueryValue, regEnumSubkeys, findExecutableOnPath, + windowsAppsAliasPrefix, + isWindowsAppExecutionAlias, + selectExecutableCandidate, mapWslDistroIcon, }; diff --git a/electron/bridges/shellDiscovery.test.cjs b/electron/bridges/shellDiscovery.test.cjs new file mode 100644 index 000000000..769bad67e --- /dev/null +++ b/electron/bridges/shellDiscovery.test.cjs @@ -0,0 +1,156 @@ +const test = require("node:test"); +const assert = require("node:assert/strict"); + +const { + isWindowsAppExecutionAlias, + selectExecutableCandidate, + windowsAppsAliasPrefix, +} = require("./shellDiscovery.cjs"); + +const WIN_ENV = { LOCALAPPDATA: "C:\\Users\\u\\AppData\\Local" }; +const ALIAS_DIR = "C:\\Users\\u\\AppData\\Local\\Microsoft\\WindowsApps"; +const ALIAS_PWSH = ALIAS_DIR + "\\pwsh.exe"; + +/** + * Build a fake fs module simulating Windows file semantics: + * - "file": regular file — existsSync/statSync/lstatSync all succeed. + * - "appExecLink": AppExecLink reparse point — existsSync/statSync fail + * (EACCES, nodejs/node#36790), lstatSync sees the reparse point. + * - "stub": plain zero-byte file (e.g. disabled alias) — all stat calls + * succeed but it is not launchable. + * Missing paths behave like ENOENT everywhere. + */ +const makeFakeFs = (entries) => { + const kindOf = (p) => entries.get(String(p).toLowerCase()); + const stat = (p) => { + const kind = kindOf(p); + if (!kind) { + const err = new Error("ENOENT"); + err.code = "ENOENT"; + throw err; + } + if (kind === "appExecLink") { + const err = new Error("EACCES"); + err.code = "EACCES"; + throw err; + } + return { isFile: () => true, size: 0 }; + }; + return { + existsSync: (p) => { + const kind = kindOf(p); + return !!kind && kind !== "appExecLink"; + }, + statSync: stat, + lstatSync: (p) => { + if (!kindOf(p)) { + const err = new Error("ENOENT"); + err.code = "ENOENT"; + throw err; + } + return { isFile: () => true, size: 0 }; + }, + }; +}; + +const WIN_DEPS = (entries) => ({ + fs: makeFakeFs(entries), + env: WIN_ENV, + platform: "win32", +}); + +test("windowsAppsAliasPrefix returns null off Windows", () => { + assert.equal(windowsAppsAliasPrefix(WIN_ENV, "linux"), null); + assert.equal(windowsAppsAliasPrefix(WIN_ENV, "darwin"), null); +}); + +test("windowsAppsAliasPrefix joins LOCALAPPDATA + WindowsApps on win32", () => { + const prefix = windowsAppsAliasPrefix(WIN_ENV, "win32"); + assert.ok(prefix.startsWith("c:\\users\\u\\appdata\\local")); + assert.ok(prefix.endsWith("microsoft\\windowsapps\\")); +}); + +test("isWindowsAppExecutionAlias matches the per-user alias directory", () => { + assert.equal(isWindowsAppExecutionAlias(ALIAS_PWSH, WIN_ENV, "win32"), true); + assert.equal( + isWindowsAppExecutionAlias(ALIAS_DIR + "\\WindowsPowerShell\\pwsh.exe", WIN_ENV, "win32"), + true, + ); + assert.equal( + isWindowsAppExecutionAlias("C:\\Program Files\\PowerShell\\7\\pwsh.exe", WIN_ENV, "win32"), + false, + ); + assert.equal(isWindowsAppExecutionAlias(ALIAS_PWSH, WIN_ENV, "linux"), false); + assert.equal(isWindowsAppExecutionAlias(null, WIN_ENV, "win32"), false); +}); + +test("selectExecutableCandidate returns the first existing regular file", () => { + const deps = WIN_DEPS(new Map([ + ["c:\\tools\\pwsh.exe", "file"], + ])); + assert.equal( + selectExecutableCandidate(["C:\\tools\\pwsh.exe", ALIAS_PWSH], deps), + "C:\\tools\\pwsh.exe", + ); +}); + +test("selectExecutableCandidate skips missing candidates", () => { + const deps = WIN_DEPS(new Map()); + assert.equal(selectExecutableCandidate(["C:\\nope\\pwsh.exe"], deps), null); +}); + +test("selectExecutableCandidate accepts an MSIX app execution alias (issue #3280)", () => { + // winget MSIX PowerShell 7: only the WindowsApps alias exists, and + // existsSync()/statSync() fail on its AppExecLink reparse point. + const deps = WIN_DEPS(new Map([[ALIAS_PWSH.toLowerCase(), "appExecLink"]])); + assert.equal(selectExecutableCandidate([ALIAS_PWSH], deps), ALIAS_PWSH); +}); + +test("selectExecutableCandidate rejects a plain zero-byte alias stub", () => { + // Disabled alias: statSync succeeds (plain file) → not launchable. + const deps = WIN_DEPS(new Map([[ALIAS_PWSH.toLowerCase(), "stub"]])); + assert.equal(selectExecutableCandidate([ALIAS_PWSH], deps), null); +}); + +test("selectExecutableCandidate prefers a real install over the alias", () => { + const deps = WIN_DEPS(new Map([ + ["c:\\program files\\powershell\\7\\pwsh.exe", "file"], + [ALIAS_PWSH.toLowerCase(), "appExecLink"], + ])); + assert.equal( + selectExecutableCandidate([ + "C:\\Program Files\\PowerShell\\7\\pwsh.exe", + ALIAS_PWSH, + ], deps), + "C:\\Program Files\\PowerShell\\7\\pwsh.exe", + ); + // Alias is still kept as fallback when it comes first in PATH order. + assert.equal( + selectExecutableCandidate([ + ALIAS_PWSH, + "C:\\Program Files\\PowerShell\\7\\pwsh.exe", + ], deps), + "C:\\Program Files\\PowerShell\\7\\pwsh.exe", + ); +}); + +test("selectExecutableCandidate falls back to the alias when no real install exists", () => { + const deps = WIN_DEPS(new Map([ + [ALIAS_PWSH.toLowerCase(), "appExecLink"], + ])); + assert.equal( + selectExecutableCandidate([ + "C:\\Program Files\\PowerShell\\7\\pwsh.exe", + ALIAS_PWSH, + ], deps), + ALIAS_PWSH, + ); +}); + +test("selectExecutableCandidate ignores empty candidates", () => { + const deps = WIN_DEPS(new Map([["c:\\tools\\pwsh.exe", "file"]])); + assert.equal( + selectExecutableCandidate(["", "C:\\tools\\pwsh.exe"], deps), + "C:\\tools\\pwsh.exe", + ); +}); diff --git a/electron/bridges/terminalBridge.cjs b/electron/bridges/terminalBridge.cjs index 0152af4a9..dea039fd4 100644 --- a/electron/bridges/terminalBridge.cjs +++ b/electron/bridges/terminalBridge.cjs @@ -58,7 +58,10 @@ const sessionLogStreamManager = require("./sessionLogStreamManager.cjs"); const { detectShellKind } = require("./ai/ptyExec.cjs"); const { stripAnsi, trackSessionIdlePrompt } = require("./ai/shellUtils.cjs"); const { createZmodemSentry } = require("./zmodemHelper.cjs"); -const { discoverShells } = require("./shellDiscovery.cjs"); +const { + discoverShells, + selectExecutableCandidate, +} = require("./shellDiscovery.cjs"); const moshHandshake = require("./moshHandshake.cjs"); const tempDirBridge = require("./tempDirBridge.cjs"); const { createTelnetAutoLogin } = require("./telnetAutoLogin.cjs"); @@ -741,22 +744,9 @@ function resolvePosixExecutable(name, opts = {}) { /** * Find executable path on Windows */ -function isWindowsAppExecutionAlias(filePath) { - if (!filePath || process.platform !== "win32") return false; - - const normalizedPath = path.normalize(filePath).toLowerCase(); - const windowsAppsDir = path.join( - process.env.LOCALAPPDATA || "", - "Microsoft", - "WindowsApps", - ).toLowerCase(); - - return !!windowsAppsDir && normalizedPath.startsWith(`${windowsAppsDir}${path.sep}`); -} - function findExecutable(name, opts = {}) { if (process.platform !== "win32") return name; - + const { execFileSync } = require("child_process"); try { const pathOverride = Object.prototype.hasOwnProperty.call(opts, "pathOverride") @@ -776,15 +766,17 @@ function findExecutable(name, opts = {}) { .map((line) => line.trim()) .filter(Boolean); - for (const candidate of candidates) { - if (!fs.existsSync(candidate)) continue; - if (name === "pwsh" && isWindowsAppExecutionAlias(candidate)) continue; - return candidate; - } + // selectExecutableCandidate accepts Windows App Execution Aliases + // (MSIX/Store installs, e.g. winget's MSIX PowerShell 7) as a fallback + // after regular files — ConPTY launches them fine via the full alias + // path even though fs.existsSync() cannot resolve AppExecLink reparse + // points (issue #3280). + const found = selectExecutableCandidate(candidates, { fs, env: process.env }); + if (found) return found; } catch (err) { console.warn(`Could not find ${name} via where.exe:`, err.message); } - + if (!/^[a-zA-Z0-9._-]+$/.test(name)) return name; const commonPaths = []; @@ -793,6 +785,9 @@ function findExecutable(name, opts = {}) { commonPaths.push( path.join(process.env.ProgramFiles || "C:\\Program Files", "PowerShell", "7", "pwsh.exe"), path.join(process.env.ProgramW6432 || "C:\\Program Files", "PowerShell", "7", "pwsh.exe"), + // MSIX/Store (winget) installs expose pwsh only via the per-user + // app execution alias — there is no "Program Files\PowerShell\7". + path.join(process.env.LOCALAPPDATA || "", "Microsoft", "WindowsApps", "pwsh.exe"), ); } @@ -815,9 +810,11 @@ function findExecutable(name, opts = {}) { ); for (const p of commonPaths) { - if (fs.existsSync(p)) return p; + // selectExecutableCandidate also accepts app execution aliases, whose + // AppExecLink reparse points fs.existsSync() cannot see. + if (selectExecutableCandidate([p], { fs, env: process.env })) return p; } - + return name; }