fix(win): prevent user PATH clobbering and normalize gateway PATH env

vcfgv · vcfgv · commit 77b79fe92166 · 2026-03-13T10:48:04.000+08:00
diff --git a/electron/gateway/config-sync.ts b/electron/gateway/config-sync.ts
@@ -11,6 +11,7 @@ import { syncGatewayTokenToConfig, syncBrowserConfigToOpenClaw, sanitizeOpenClaw
 import { buildProxyEnv, resolveProxySettings } from '../utils/proxy';
 import { syncProxyConfigToOpenClaw } from '../utils/openclaw-proxy';
 import { logger } from '../utils/logger';
+import { prependPathEntry } from '../utils/env-path';
 
 export interface GatewayLaunchContext {
   appSettings: Awaited<ReturnType<typeof getAllSettings>>;
@@ -141,9 +142,6 @@ export async function prepareGatewayLaunchContext(port: number): Promise<Gateway
     ? path.join(process.resourcesPath, 'bin')
     : path.join(process.cwd(), 'resources', 'bin', target);
   const binPathExists = existsSync(binPath);
-  const finalPath = binPathExists
-    ? `${binPath}${path.delimiter}${process.env.PATH || ''}`
-    : process.env.PATH || '';
 
   const { providerEnv, loadedProviderKeyCount } = await loadProviderEnv();
   const { skipChannels, channelStartupSummary } = await resolveChannelStartupPolicy();
@@ -155,9 +153,12 @@ export async function prepareGatewayLaunchContext(port: number): Promise<Gateway
     : 'disabled';
 
   const { NODE_OPTIONS: _nodeOptions, ...baseEnv } = process.env;
+  const baseEnvRecord = baseEnv as Record<string, string | undefined>;
+  const baseEnvPatched = binPathExists
+    ? prependPathEntry(baseEnvRecord, binPath).env
+    : baseEnvRecord;
   const forkEnv: Record<string, string | undefined> = {
-    ...baseEnv,
-    PATH: finalPath,
+    ...baseEnvPatched,
     ...providerEnv,
     ...uvEnv,
     ...proxyEnv,
diff --git a/electron/gateway/supervisor.ts b/electron/gateway/supervisor.ts
@@ -6,6 +6,7 @@ import { getOpenClawDir, getOpenClawEntryPath } from '../utils/paths';
 import { getUvMirrorEnv } from '../utils/uv-env';
 import { isPythonReady, setupManagedPython } from '../utils/uv-setup';
 import { logger } from '../utils/logger';
+import { prependPathEntry } from '../utils/env-path';
 
 export function warmupManagedPythonReadiness(): void {
   void isPythonReady().then((pythonReady) => {
@@ -269,9 +270,10 @@ export async function runOpenClawDoctorRepair(): Promise<boolean> {
     ? path.join(process.resourcesPath, 'bin')
     : path.join(process.cwd(), 'resources', 'bin', target);
   const binPathExists = existsSync(binPath);
-  const finalPath = binPathExists
-    ? `${binPath}${path.delimiter}${process.env.PATH || ''}`
-    : process.env.PATH || '';
+  const baseProcessEnv = process.env as Record<string, string | undefined>;
+  const baseEnvPatched = binPathExists
+    ? prependPathEntry(baseProcessEnv, binPath).env
+    : baseProcessEnv;
 
   const uvEnv = await getUvMirrorEnv();
   const doctorArgs = ['doctor', '--fix', '--yes', '--non-interactive'];
@@ -281,8 +283,7 @@ export async function runOpenClawDoctorRepair(): Promise<boolean> {
 
   return await new Promise<boolean>((resolve) => {
     const forkEnv: Record<string, string | undefined> = {
-      ...process.env,
-      PATH: finalPath,
+      ...baseEnvPatched,
       ...uvEnv,
       OPENCLAW_NO_RESPAWN: '1',
     };
diff --git a/electron/utils/env-path.ts b/electron/utils/env-path.ts
@@ -0,0 +1,59 @@
+type EnvMap = Record<string, string | undefined>;
+
+function isPathKey(key: string): boolean {
+  return key.toLowerCase() === 'path';
+}
+
+function preferredPathKey(): string {
+  return process.platform === 'win32' ? 'Path' : 'PATH';
+}
+
+function pathDelimiter(): string {
+  return process.platform === 'win32' ? ';' : ':';
+}
+
+export function getPathEnvKey(env: EnvMap): string {
+  const keys = Object.keys(env).filter(isPathKey);
+  if (keys.length === 0) return preferredPathKey();
+
+  if (process.platform === 'win32') {
+    if (keys.includes('Path')) return 'Path';
+    if (keys.includes('PATH')) return 'PATH';
+    return keys[0];
+  }
+
+  if (keys.includes('PATH')) return 'PATH';
+  return keys[0];
+}
+
+export function getPathEnvValue(env: EnvMap): string {
+  const key = getPathEnvKey(env);
+  return env[key] ?? '';
+}
+
+export function setPathEnvValue(
+  env: EnvMap,
+  nextPath: string,
+): EnvMap {
+  const nextEnv: EnvMap = { ...env };
+  for (const key of Object.keys(nextEnv)) {
+    if (isPathKey(key)) {
+      delete nextEnv[key];
+    }
+  }
+
+  nextEnv[getPathEnvKey(env)] = nextPath;
+  return nextEnv;
+}
+
+export function prependPathEntry(
+  env: EnvMap,
+  entry: string,
+): { env: EnvMap; path: string } {
+  const current = getPathEnvValue(env);
+  const nextPath = current ? `${entry}${pathDelimiter()}${current}` : entry;
+  return {
+    env: setPathEnvValue(env, nextPath),
+    path: nextPath,
+  };
+}
diff --git a/resources/cli/win32/update-user-path.ps1 b/resources/cli/win32/update-user-path.ps1
@@ -9,6 +9,39 @@ param(
 
 $ErrorActionPreference = 'Stop'
 
+function Get-UserPathRegistryValue {
+  $raw = [Environment]::GetEnvironmentVariable('Path', 'User')
+  $kind = [Microsoft.Win32.RegistryValueKind]::ExpandString
+
+  try {
+    $key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment', $false)
+    if ($null -ne $key) {
+      try {
+        $stored = $key.GetValue('Path', $null, [Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames)
+        if ($null -ne $stored) {
+          $raw = [string]$stored
+        }
+      } catch {
+        # Fallback to Environment API value
+      }
+
+      try {
+        $kind = $key.GetValueKind('Path')
+      } catch {
+        # Keep default ExpandString
+      }
+      $key.Close()
+    }
+  } catch {
+    # Fallback to Environment API value
+  }
+
+  return @{
+    Raw = $raw
+    Kind = $kind
+  }
+}
+
 function Normalize-PathEntry {
   param([string]$Value)
 
@@ -19,7 +52,8 @@ function Normalize-PathEntry {
   return $Value.Trim().Trim('"').TrimEnd('\').ToLowerInvariant()
 }
 
-$current = [Environment]::GetEnvironmentVariable('Path', 'User')
+$pathMeta = Get-UserPathRegistryValue
+$current = $pathMeta.Raw
 $entries = @()
 if (-not [string]::IsNullOrWhiteSpace($current)) {
   $entries = $current -split ';' | Where-Object { -not [string]::IsNullOrWhiteSpace($_) }
@@ -54,35 +88,63 @@ if ($Action -eq 'add') {
   $status = 'updated'
 }
 
+$isLikelyCorruptedWrite = (
+  $Action -eq 'add' -and
+  $entries.Count -gt 1 -and
+  $nextEntries.Count -le 1
+)
+if ($isLikelyCorruptedWrite) {
+  throw "Refusing to rewrite user PATH: input had $($entries.Count) entries but output has $($nextEntries.Count)."
+}
+
 $newPath = if ($nextEntries.Count -eq 0) { $null } else { $nextEntries -join ';' }
-[Environment]::SetEnvironmentVariable('Path', $newPath, 'User')
-
-Add-Type -Namespace OpenClaw -Name NativeMethods -MemberDefinition @"
-using System;
-using System.Runtime.InteropServices;
-public static class NativeMethods {
-  [DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Auto)]
-  public static extern IntPtr SendMessageTimeout(
-    IntPtr hWnd,
-    int Msg,
-    IntPtr wParam,
-    string lParam,
-    int fuFlags,
-    int uTimeout,
-    out IntPtr lpdwResult
-  );
+try {
+  $key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment', $true)
+  if ($null -eq $key) {
+    throw 'Unable to open HKCU\Environment for write.'
+  }
+
+  if ([string]::IsNullOrWhiteSpace($newPath)) {
+    $key.DeleteValue('Path', $false)
+  } else {
+    $kind = if ($pathMeta.Kind -eq [Microsoft.Win32.RegistryValueKind]::String) {
+      [Microsoft.Win32.RegistryValueKind]::String
+    } else {
+      [Microsoft.Win32.RegistryValueKind]::ExpandString
+    }
+    $key.SetValue('Path', $newPath, $kind)
+  }
+  $key.Close()
+} catch {
+  throw "Failed to write HKCU\\Environment\\Path: $($_.Exception.Message)"
 }
+
+try {
+  Add-Type -Namespace OpenClaw -Name NativeMethods -MemberDefinition @"
+[System.Runtime.InteropServices.DllImport("user32.dll", SetLastError = true, CharSet = System.Runtime.InteropServices.CharSet.Auto)]
+public static extern System.IntPtr SendMessageTimeout(
+  System.IntPtr hWnd,
+  int Msg,
+  System.IntPtr wParam,
+  string lParam,
+  int fuFlags,
+  int uTimeout,
+  out System.IntPtr lpdwResult
+);
 "@
 
-$result = [IntPtr]::Zero
-[OpenClaw.NativeMethods]::SendMessageTimeout(
-  [IntPtr]0xffff,
-  0x001A,
-  [IntPtr]::Zero,
-  'Environment',
-  0x0002,
-  5000,
-  [ref]$result
-) | Out-Null
+  $result = [IntPtr]::Zero
+  [OpenClaw.NativeMethods]::SendMessageTimeout(
+    [IntPtr]0xffff,
+    0x001A,
+    [IntPtr]::Zero,
+    'Environment',
+    0x0002,
+    5000,
+    [ref]$result
+  ) | Out-Null
+} catch {
+  Write-Warning "PATH updated but failed to broadcast environment change: $($_.Exception.Message)"
+}
 
 Write-Output $status
diff --git a/tests/unit/env-path.test.ts b/tests/unit/env-path.test.ts
@@ -0,0 +1,73 @@
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+
+const originalPlatform = process.platform;
+
+function setPlatform(platform: string) {
+  Object.defineProperty(process, 'platform', { value: platform, writable: true });
+}
+
+afterEach(() => {
+  Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true });
+});
+
+describe('env-path', () => {
+  let getPathEnvKey: (env: Record<string, string | undefined>) => string;
+  let getPathEnvValue: (env: Record<string, string | undefined>) => string;
+  let setPathEnvValue: (
+    env: Record<string, string | undefined>,
+    nextPath: string,
+  ) => Record<string, string | undefined>;
+  let prependPathEntry: (
+    env: Record<string, string | undefined>,
+    entry: string,
+  ) => { env: Record<string, string | undefined>; path: string };
+
+  beforeEach(async () => {
+    const mod = await import('@electron/utils/env-path');
+    getPathEnvKey = mod.getPathEnvKey;
+    getPathEnvValue = mod.getPathEnvValue;
+    setPathEnvValue = mod.setPathEnvValue;
+    prependPathEntry = mod.prependPathEntry;
+  });
+
+  it('prefers Path key on Windows', () => {
+    setPlatform('win32');
+    expect(getPathEnvKey({ Path: 'C:\\Windows', PATH: 'C:\\Temp' })).toBe('Path');
+  });
+
+  it('reads path value from Path key on Windows', () => {
+    setPlatform('win32');
+    expect(getPathEnvValue({ Path: 'C:\\Windows;C:\\Tools' })).toBe('C:\\Windows;C:\\Tools');
+  });
+
+  it('uses PATH key on non-Windows', () => {
+    setPlatform('linux');
+    expect(getPathEnvKey({ PATH: '/usr/bin', Path: '/tmp/bin' })).toBe('PATH');
+  });
+
+  it('removes duplicate path keys when setting a new value', () => {
+    setPlatform('win32');
+    const next = setPathEnvValue(
+      { Path: 'C:\\A', PATH: 'C:\\B', PaTh: 'C:\\C', HOME: 'C:\\Users\\me' },
+      'C:\\A;C:\\B',
+    );
+    expect(next.Path).toBe('C:\\A;C:\\B');
+    expect(next.PATH).toBeUndefined();
+    expect(next.PaTh).toBeUndefined();
+    expect(next.HOME).toBe('C:\\Users\\me');
+  });
+
+  it('prepends entry with Windows delimiter', () => {
+    setPlatform('win32');
+    const next = prependPathEntry({ Path: 'C:\\Windows\\System32' }, 'D:\\clawx\\resources\\bin');
+    expect(next.path).toBe('D:\\clawx\\resources\\bin;C:\\Windows\\System32');
+    expect(next.env.Path).toBe('D:\\clawx\\resources\\bin;C:\\Windows\\System32');
+  });
+
+  it('prepends entry with POSIX delimiter', () => {
+    setPlatform('linux');
+    const next = prependPathEntry({ PATH: '/usr/bin:/bin' }, '/opt/clawx/bin');
+    expect(next.path).toBe('/opt/clawx/bin:/usr/bin:/bin');
+    expect(next.env.PATH).toBe('/opt/clawx/bin:/usr/bin:/bin');
+  });
+});
