From 740116ae9d12834fc3340151467e479afeb345fe Mon Sep 17 00:00:00 2001 From: Felix <24791380+vcfgv@users.noreply.github.com> Date: Fri, 13 Mar 2026 12:12:30 +0800 Subject: [PATCH] fix(win): prevent user PATH clobbering and normalize gateway PATH env (#459) --- .github/workflows/package-win-manual.yml | 31 +++++- electron/gateway/config-sync.ts | 11 ++- electron/gateway/supervisor.ts | 11 ++- electron/utils/env-path.ts | 59 ++++++++++++ resources/cli/win32/update-user-path.ps1 | 118 +++++++++++++++++------ tests/unit/env-path.test.ts | 73 ++++++++++++++ 6 files changed, 261 insertions(+), 42 deletions(-) create mode 100644 electron/utils/env-path.ts create mode 100644 tests/unit/env-path.test.ts diff --git a/.github/workflows/package-win-manual.yml b/.github/workflows/package-win-manual.yml index d3954881a..ad72de078 100644 --- a/.github/workflows/package-win-manual.yml +++ b/.github/workflows/package-win-manual.yml @@ -54,13 +54,36 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: pnpm run build:vite && pnpm exec zx scripts/bundle-openclaw.mjs && pnpm exec electron-builder --win --publish never - - name: Upload Windows artifacts + - name: Upload Windows Installer (x64) uses: actions/upload-artifact@v4 with: - name: windows-package + name: windows-installer-x64 + path: release/*-win-x64.exe + if-no-files-found: error + retention-days: 7 + + - name: Upload Windows Installer (arm64) + uses: actions/upload-artifact@v4 + with: + name: windows-installer-arm64 + path: release/*-win-arm64.exe + if-no-files-found: warn + retention-days: 7 + + - name: Upload Windows Blockmap Files + uses: actions/upload-artifact@v4 + with: + name: windows-blockmap + path: release/*.blockmap + if-no-files-found: warn + retention-days: 7 + + - name: Upload Windows Update Manifests + uses: actions/upload-artifact@v4 + with: + name: windows-update-manifests path: | - release/*.exe - release/*.blockmap release/*.yml !release/builder-debug.yml + if-no-files-found: warn retention-days: 7 diff --git a/electron/gateway/config-sync.ts b/electron/gateway/config-sync.ts index 6dadb74ea..0e9ecab60 100644 --- 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>; @@ -141,9 +142,6 @@ export async function prepareGatewayLaunchContext(port: number): Promise; + const baseEnvPatched = binPathExists + ? prependPathEntry(baseEnvRecord, binPath).env + : baseEnvRecord; const forkEnv: Record = { - ...baseEnv, - PATH: finalPath, + ...baseEnvPatched, ...providerEnv, ...uvEnv, ...proxyEnv, diff --git a/electron/gateway/supervisor.ts b/electron/gateway/supervisor.ts index 5379b565f..04391d944 100644 --- 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 { ? 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; + 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 { return await new Promise((resolve) => { const forkEnv: Record = { - ...process.env, - PATH: finalPath, + ...baseEnvPatched, ...uvEnv, OPENCLAW_NO_RESPAWN: '1', }; diff --git a/electron/utils/env-path.ts b/electron/utils/env-path.ts new file mode 100644 index 000000000..4b8b94479 --- /dev/null +++ b/electron/utils/env-path.ts @@ -0,0 +1,59 @@ +type EnvMap = Record; + +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 index 99677039f..3b5204b84 100644 --- 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' } -$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 - ); +$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 ';' } +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 new file mode 100644 index 000000000..1d9bdd8fe --- /dev/null +++ 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; + let getPathEnvValue: (env: Record) => string; + let setPathEnvValue: ( + env: Record, + nextPath: string, + ) => Record; + let prependPathEntry: ( + env: Record, + entry: string, + ) => { env: Record; 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'); + }); +});