Merge pull request #209 from pylon-code/upstream/2026-08-31-windows-path-quotes

fix(windows): strip quotes from repaired PATH

Author: rynfar

apps/desktop/src/shell/DesktopShellEnvironment.test.ts

@@ -334,7 +334,7 @@ describe("DesktopShellEnvironment", () => {
                 FNM_DIR: "C:\\Users\\testuser\\AppData\\Roaming\\fnm",
                 FNM_MULTISHELL_PATH: "C:\\Users\\testuser\\AppData\\Local\\fnm_multishells\\123",
               })
-            : envOutput({ PATH: "C:\\Custom\\Bin;C:\\Windows\\System32" });
+            : envOutput({ PATH: 'C:\\Custom\\Bin;C:";C:\\Windows\\System32' });
         },
       });
 

apps/desktop/src/shell/DesktopShellEnvironment.ts

@@ -151,6 +151,50 @@ const pathComparisonKey = (entry: string, platform: NodeJS.Platform) => {
   return platform === "win32" ? normalized.toLowerCase() : normalized;
 };
 
+const sanitizePathEntry = (entry: string, platform: NodeJS.Platform) =>
+  platform === "win32" ? entry.replaceAll('"', "") : entry;
+
+/**
+ * Splits a PATH value, honouring Windows quoting. A quoted entry may contain the
+ * delimiter — `C:\\bin;"C:\\my;dir"` is two directories — so splitting on the raw
+ * delimiter would tear it apart and leave a relative `dir` entry behind.
+ */
+const splitPathValue = (value: string, delimiter: string, platform: NodeJS.Platform): string[] => {
+  if (platform !== "win32") return value.split(delimiter);
+  // Unbalanced quotes are stray characters, not quoting: honouring them would
+  // swallow every later entry into one. Fall back to a plain split there.
+  if ((value.match(/"/g)?.length ?? 0) % 2 !== 0) return value.split(delimiter);
+  const entries: string[] = [];
+  let current = "";
+  let quoted = false;
+  for (const char of value) {
+    if (char === '"') {
+      quoted = !quoted;
+      current += char;
+      continue;
+    }
+    if (char === delimiter && !quoted) {
+      entries.push(current);
+      current = "";
+      continue;
+    }
+    current += char;
+  }
+  entries.push(current);
+  return entries;
+};
+
+/**
+ * Re-quotes an entry containing the delimiter. Stripping its quotes and joining
+ * would hand consumers a value they split back into the wrong directories.
+ */
+const quotePathEntryIfNeeded = (entry: string, delimiter: string, platform: NodeJS.Platform) =>
+  platform === "win32" && entry.includes(delimiter) ? `"${entry}"` : entry;
+
+/** A bare drive letter is drive-relative, so it would add the child's cwd to the search. */
+const isUsablePathEntry = (entry: string, platform: NodeJS.Platform) =>
+  platform !== "win32" || !/^[A-Za-z]:$/.test(entry);
+
 const mergePaths = (
   platform: NodeJS.Platform,
   values: ReadonlyArray<Option.Option<string>>,
@@ -162,19 +206,23 @@ const mergePaths = (
   for (const value of values) {
     if (Option.isNone(value)) continue;
 
-    for (const entry of value.value.split(delimiter)) {
-      const trimmed = entry.trim();
-      if (trimmed.length === 0) continue;
+    for (const entry of splitPathValue(value.value, delimiter, platform)) {
+      const sanitized = sanitizePathEntry(entry.trim(), platform);
+      if (sanitized.length === 0 || !isUsablePathEntry(sanitized, platform)) continue;
 
-      const key = pathComparisonKey(trimmed, platform);
+      const key = pathComparisonKey(sanitized, platform);
       if (key.length === 0 || seen.has(key)) continue;
 
       seen.add(key);
-      entries.push(trimmed);
+      entries.push(sanitized);
     }
   }
 
-  return entries.length > 0 ? Option.some(entries.join(delimiter)) : Option.none();
+  return entries.length > 0
+    ? Option.some(
+        entries.map((entry) => quotePathEntryIfNeeded(entry, delimiter, platform)).join(delimiter),
+      )
+    : Option.none();
 };
 
 export const resolveDesktopLoginShellCandidates = (

packages/shared/src/shell.test.ts

@@ -304,15 +304,35 @@ describe("readEnvironmentFromWindowsShell", () => {
 });
 
 describe("mergePathValues", () => {
-  it("dedupes case-insensitively on Windows while preserving preferred order", () => {
+  it("sanitizes and dedupes Windows entries while preserving preferred order", () => {
     expect(
       mergePathValues(
         'C:\\Users\\testuser\\AppData\\Roaming\\npm;"C:\\Program Files\\nodejs"',
         "c:\\users\\testuser\\appdata\\roaming\\npm;C:\\Windows\\System32",
         "win32",
       ),
     ).toBe(
-      'C:\\Users\\testuser\\AppData\\Roaming\\npm;"C:\\Program Files\\nodejs";C:\\Windows\\System32',
+      "C:\\Users\\testuser\\AppData\\Roaming\\npm;C:\\Program Files\\nodejs;C:\\Windows\\System32",
+    );
+  });
+
+  it("removes stray quotes from Windows entries", () => {
+    expect(
+      mergePathValues(
+        'C:\\Windows\\System32;C:\\cloudflared.exe;C:";C:\\Program Files\\nodejs',
+        undefined,
+        "win32",
+      ),
+      // The bare drive letter left behind is drive-relative: keeping it would add
+      // the child process's own directory to the executable search.
+    ).toBe("C:\\Windows\\System32;C:\\cloudflared.exe;C:\\Program Files\\nodejs");
+  });
+
+  it("keeps a quoted Windows entry that contains the delimiter", () => {
+    // The entry stays quoted on the way out, or consumers would split it back
+    // into "C:\\my" and a relative "dir".
+    expect(mergePathValues('C:\\bin;"C:\\my;dir";C:\\other', undefined, "win32")).toBe(
+      'C:\\bin;"C:\\my;dir";C:\\other',
     );
   });
 

packages/shared/src/shell.ts

@@ -414,6 +414,58 @@ function normalizePathEntryForComparison(entry: string, platform: NodeJS.Platfor
   return platform === "win32" ? normalized.toLowerCase() : normalized;
 }
 
+function sanitizePathEntry(entry: string, platform: NodeJS.Platform): string {
+  return platform === "win32" ? entry.replaceAll('"', "") : entry;
+}
+
+/**
+ * Splits a PATH value, honouring Windows quoting. A quoted entry may itself
+ * contain the delimiter — `C:\\bin;"C:\\my;dir"` is two directories, not three —
+ * so splitting on the raw delimiter first would tear it in half and leave a
+ * relative `dir` entry resolved against the child process's cwd.
+ */
+function splitPathValue(value: string, delimiter: string, platform: NodeJS.Platform): string[] {
+  if (platform !== "win32") return value.split(delimiter);
+  // Unbalanced quotes are stray characters, not quoting: honouring them would
+  // swallow every later entry into one. Fall back to a plain split there.
+  if ((value.match(/"/g)?.length ?? 0) % 2 !== 0) return value.split(delimiter);
+  const entries: string[] = [];
+  let current = "";
+  let quoted = false;
+  for (const char of value) {
+    if (char === '"') {
+      quoted = !quoted;
+      current += char;
+      continue;
+    }
+    if (char === delimiter && !quoted) {
+      entries.push(current);
+      current = "";
+      continue;
+    }
+    current += char;
+  }
+  entries.push(current);
+  return entries;
+}
+
+/**
+ * Re-quotes an entry containing the delimiter. Stripping its quotes and joining
+ * would hand consumers a value they split back into the wrong directories.
+ */
+function quotePathEntryIfNeeded(
+  entry: string,
+  delimiter: string,
+  platform: NodeJS.Platform,
+): string {
+  return platform === "win32" && entry.includes(delimiter) ? `"${entry}"` : entry;
+}
+
+/** A bare drive letter is drive-relative, so it would add the child's cwd to the search. */
+function isUsablePathEntry(entry: string, platform: NodeJS.Platform): boolean {
+  return platform !== "win32" || !/^[A-Za-z]:$/.test(entry);
+}
+
 export function mergePathValues(
   preferredPath: string | undefined,
   inheritedPath: string | undefined,
@@ -426,19 +478,21 @@ export function mergePathValues(
   for (const rawValue of [preferredPath, inheritedPath]) {
     if (!rawValue) continue;
 
-    for (const entry of rawValue.split(delimiter)) {
-      const trimmed = entry.trim();
-      if (trimmed.length === 0) continue;
+    for (const entry of splitPathValue(rawValue, delimiter, platform)) {
+      const sanitized = sanitizePathEntry(entry.trim(), platform);
+      if (sanitized.length === 0 || !isUsablePathEntry(sanitized, platform)) continue;
 
-      const normalized = normalizePathEntryForComparison(trimmed, platform);
+      const normalized = normalizePathEntryForComparison(sanitized, platform);
       if (normalized.length === 0 || seen.has(normalized)) continue;
 
       seen.add(normalized);
-      merged.push(trimmed);
+      merged.push(sanitized);
     }
   }
 
-  return merged.length > 0 ? merged.join(delimiter) : undefined;
+  return merged.length > 0
+    ? merged.map((entry) => quotePathEntryIfNeeded(entry, delimiter, platform)).join(delimiter)
+    : undefined;
 }
 
 function readEnvPath(env: NodeJS.ProcessEnv): string | undefined {