Skip to content

Commit 740116a

Browse files
authored
fix(win): prevent user PATH clobbering and normalize gateway PATH env (#459)
1 parent 5e88022 commit 740116a

File tree

6 files changed

+260
-41
lines changed

6 files changed

+260
-41
lines changed

.github/workflows/package-win-manual.yml

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,36 @@ jobs:
5454
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
5555
run: pnpm run build:vite && pnpm exec zx scripts/bundle-openclaw.mjs && pnpm exec electron-builder --win --publish never
5656

57-
- name: Upload Windows artifacts
57+
- name: Upload Windows Installer (x64)
5858
uses: actions/upload-artifact@v4
5959
with:
60-
name: windows-package
60+
name: windows-installer-x64
61+
path: release/*-win-x64.exe
62+
if-no-files-found: error
63+
retention-days: 7
64+
65+
- name: Upload Windows Installer (arm64)
66+
uses: actions/upload-artifact@v4
67+
with:
68+
name: windows-installer-arm64
69+
path: release/*-win-arm64.exe
70+
if-no-files-found: warn
71+
retention-days: 7
72+
73+
- name: Upload Windows Blockmap Files
74+
uses: actions/upload-artifact@v4
75+
with:
76+
name: windows-blockmap
77+
path: release/*.blockmap
78+
if-no-files-found: warn
79+
retention-days: 7
80+
81+
- name: Upload Windows Update Manifests
82+
uses: actions/upload-artifact@v4
83+
with:
84+
name: windows-update-manifests
6185
path: |
62-
release/*.exe
63-
release/*.blockmap
6486
release/*.yml
6587
!release/builder-debug.yml
88+
if-no-files-found: warn
6689
retention-days: 7

electron/gateway/config-sync.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { syncGatewayTokenToConfig, syncBrowserConfigToOpenClaw, sanitizeOpenClaw
1111
import { buildProxyEnv, resolveProxySettings } from '../utils/proxy';
1212
import { syncProxyConfigToOpenClaw } from '../utils/openclaw-proxy';
1313
import { logger } from '../utils/logger';
14+
import { prependPathEntry } from '../utils/env-path';
1415

1516
export interface GatewayLaunchContext {
1617
appSettings: Awaited<ReturnType<typeof getAllSettings>>;
@@ -141,9 +142,6 @@ export async function prepareGatewayLaunchContext(port: number): Promise<Gateway
141142
? path.join(process.resourcesPath, 'bin')
142143
: path.join(process.cwd(), 'resources', 'bin', target);
143144
const binPathExists = existsSync(binPath);
144-
const finalPath = binPathExists
145-
? `${binPath}${path.delimiter}${process.env.PATH || ''}`
146-
: process.env.PATH || '';
147145

148146
const { providerEnv, loadedProviderKeyCount } = await loadProviderEnv();
149147
const { skipChannels, channelStartupSummary } = await resolveChannelStartupPolicy();
@@ -155,9 +153,12 @@ export async function prepareGatewayLaunchContext(port: number): Promise<Gateway
155153
: 'disabled';
156154

157155
const { NODE_OPTIONS: _nodeOptions, ...baseEnv } = process.env;
156+
const baseEnvRecord = baseEnv as Record<string, string | undefined>;
157+
const baseEnvPatched = binPathExists
158+
? prependPathEntry(baseEnvRecord, binPath).env
159+
: baseEnvRecord;
158160
const forkEnv: Record<string, string | undefined> = {
159-
...baseEnv,
160-
PATH: finalPath,
161+
...baseEnvPatched,
161162
...providerEnv,
162163
...uvEnv,
163164
...proxyEnv,

electron/gateway/supervisor.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getOpenClawDir, getOpenClawEntryPath } from '../utils/paths';
66
import { getUvMirrorEnv } from '../utils/uv-env';
77
import { isPythonReady, setupManagedPython } from '../utils/uv-setup';
88
import { logger } from '../utils/logger';
9+
import { prependPathEntry } from '../utils/env-path';
910

1011
export function warmupManagedPythonReadiness(): void {
1112
void isPythonReady().then((pythonReady) => {
@@ -269,9 +270,10 @@ export async function runOpenClawDoctorRepair(): Promise<boolean> {
269270
? path.join(process.resourcesPath, 'bin')
270271
: path.join(process.cwd(), 'resources', 'bin', target);
271272
const binPathExists = existsSync(binPath);
272-
const finalPath = binPathExists
273-
? `${binPath}${path.delimiter}${process.env.PATH || ''}`
274-
: process.env.PATH || '';
273+
const baseProcessEnv = process.env as Record<string, string | undefined>;
274+
const baseEnvPatched = binPathExists
275+
? prependPathEntry(baseProcessEnv, binPath).env
276+
: baseProcessEnv;
275277

276278
const uvEnv = await getUvMirrorEnv();
277279
const doctorArgs = ['doctor', '--fix', '--yes', '--non-interactive'];
@@ -281,8 +283,7 @@ export async function runOpenClawDoctorRepair(): Promise<boolean> {
281283

282284
return await new Promise<boolean>((resolve) => {
283285
const forkEnv: Record<string, string | undefined> = {
284-
...process.env,
285-
PATH: finalPath,
286+
...baseEnvPatched,
286287
...uvEnv,
287288
OPENCLAW_NO_RESPAWN: '1',
288289
};

electron/utils/env-path.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
type EnvMap = Record<string, string | undefined>;
2+
3+
function isPathKey(key: string): boolean {
4+
return key.toLowerCase() === 'path';
5+
}
6+
7+
function preferredPathKey(): string {
8+
return process.platform === 'win32' ? 'Path' : 'PATH';
9+
}
10+
11+
function pathDelimiter(): string {
12+
return process.platform === 'win32' ? ';' : ':';
13+
}
14+
15+
export function getPathEnvKey(env: EnvMap): string {
16+
const keys = Object.keys(env).filter(isPathKey);
17+
if (keys.length === 0) return preferredPathKey();
18+
19+
if (process.platform === 'win32') {
20+
if (keys.includes('Path')) return 'Path';
21+
if (keys.includes('PATH')) return 'PATH';
22+
return keys[0];
23+
}
24+
25+
if (keys.includes('PATH')) return 'PATH';
26+
return keys[0];
27+
}
28+
29+
export function getPathEnvValue(env: EnvMap): string {
30+
const key = getPathEnvKey(env);
31+
return env[key] ?? '';
32+
}
33+
34+
export function setPathEnvValue(
35+
env: EnvMap,
36+
nextPath: string,
37+
): EnvMap {
38+
const nextEnv: EnvMap = { ...env };
39+
for (const key of Object.keys(nextEnv)) {
40+
if (isPathKey(key)) {
41+
delete nextEnv[key];
42+
}
43+
}
44+
45+
nextEnv[getPathEnvKey(env)] = nextPath;
46+
return nextEnv;
47+
}
48+
49+
export function prependPathEntry(
50+
env: EnvMap,
51+
entry: string,
52+
): { env: EnvMap; path: string } {
53+
const current = getPathEnvValue(env);
54+
const nextPath = current ? `${entry}${pathDelimiter()}${current}` : entry;
55+
return {
56+
env: setPathEnvValue(env, nextPath),
57+
path: nextPath,
58+
};
59+
}

resources/cli/win32/update-user-path.ps1

Lines changed: 89 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,39 @@ param(
99

1010
$ErrorActionPreference = 'Stop'
1111

12+
function Get-UserPathRegistryValue {
13+
$raw = [Environment]::GetEnvironmentVariable('Path', 'User')
14+
$kind = [Microsoft.Win32.RegistryValueKind]::ExpandString
15+
16+
try {
17+
$key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment', $false)
18+
if ($null -ne $key) {
19+
try {
20+
$stored = $key.GetValue('Path', $null, [Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames)
21+
if ($null -ne $stored) {
22+
$raw = [string]$stored
23+
}
24+
} catch {
25+
# Fallback to Environment API value
26+
}
27+
28+
try {
29+
$kind = $key.GetValueKind('Path')
30+
} catch {
31+
# Keep default ExpandString
32+
}
33+
$key.Close()
34+
}
35+
} catch {
36+
# Fallback to Environment API value
37+
}
38+
39+
return @{
40+
Raw = $raw
41+
Kind = $kind
42+
}
43+
}
44+
1245
function Normalize-PathEntry {
1346
param([string]$Value)
1447

@@ -19,7 +52,8 @@ function Normalize-PathEntry {
1952
return $Value.Trim().Trim('"').TrimEnd('\').ToLowerInvariant()
2053
}
2154

22-
$current = [Environment]::GetEnvironmentVariable('Path', 'User')
55+
$pathMeta = Get-UserPathRegistryValue
56+
$current = $pathMeta.Raw
2357
$entries = @()
2458
if (-not [string]::IsNullOrWhiteSpace($current)) {
2559
$entries = $current -split ';' | Where-Object { -not [string]::IsNullOrWhiteSpace($_) }
@@ -54,35 +88,63 @@ if ($Action -eq 'add') {
5488
$status = 'updated'
5589
}
5690

91+
$isLikelyCorruptedWrite = (
92+
$Action -eq 'add' -and
93+
$entries.Count -gt 1 -and
94+
$nextEntries.Count -le 1
95+
)
96+
if ($isLikelyCorruptedWrite) {
97+
throw "Refusing to rewrite user PATH: input had $($entries.Count) entries but output has $($nextEntries.Count)."
98+
}
99+
57100
$newPath = if ($nextEntries.Count -eq 0) { $null } else { $nextEntries -join ';' }
58-
[Environment]::SetEnvironmentVariable('Path', $newPath, 'User')
59-
60-
Add-Type -Namespace OpenClaw -Name NativeMethods -MemberDefinition @"
61-
using System;
62-
using System.Runtime.InteropServices;
63-
public static class NativeMethods {
64-
[DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Auto)]
65-
public static extern IntPtr SendMessageTimeout(
66-
IntPtr hWnd,
67-
int Msg,
68-
IntPtr wParam,
69-
string lParam,
70-
int fuFlags,
71-
int uTimeout,
72-
out IntPtr lpdwResult
73-
);
101+
try {
102+
$key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment', $true)
103+
if ($null -eq $key) {
104+
throw 'Unable to open HKCU\Environment for write.'
105+
}
106+
107+
if ([string]::IsNullOrWhiteSpace($newPath)) {
108+
$key.DeleteValue('Path', $false)
109+
} else {
110+
$kind = if ($pathMeta.Kind -eq [Microsoft.Win32.RegistryValueKind]::String) {
111+
[Microsoft.Win32.RegistryValueKind]::String
112+
} else {
113+
[Microsoft.Win32.RegistryValueKind]::ExpandString
114+
}
115+
$key.SetValue('Path', $newPath, $kind)
116+
}
117+
$key.Close()
118+
} catch {
119+
throw "Failed to write HKCU\\Environment\\Path: $($_.Exception.Message)"
74120
}
121+
122+
try {
123+
Add-Type -Namespace OpenClaw -Name NativeMethods -MemberDefinition @"
124+
[System.Runtime.InteropServices.DllImport("user32.dll", SetLastError = true, CharSet = System.Runtime.InteropServices.CharSet.Auto)]
125+
public static extern System.IntPtr SendMessageTimeout(
126+
System.IntPtr hWnd,
127+
int Msg,
128+
System.IntPtr wParam,
129+
string lParam,
130+
int fuFlags,
131+
int uTimeout,
132+
out System.IntPtr lpdwResult
133+
);
75134
"@
76135

77-
$result = [IntPtr]::Zero
78-
[OpenClaw.NativeMethods]::SendMessageTimeout(
79-
[IntPtr]0xffff,
80-
0x001A,
81-
[IntPtr]::Zero,
82-
'Environment',
83-
0x0002,
84-
5000,
85-
[ref]$result
86-
) | Out-Null
136+
$result = [IntPtr]::Zero
137+
[OpenClaw.NativeMethods]::SendMessageTimeout(
138+
[IntPtr]0xffff,
139+
0x001A,
140+
[IntPtr]::Zero,
141+
'Environment',
142+
0x0002,
143+
5000,
144+
[ref]$result
145+
) | Out-Null
146+
} catch {
147+
Write-Warning "PATH updated but failed to broadcast environment change: $($_.Exception.Message)"
148+
}
87149

88150
Write-Output $status

tests/unit/env-path.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
2+
3+
const originalPlatform = process.platform;
4+
5+
function setPlatform(platform: string) {
6+
Object.defineProperty(process, 'platform', { value: platform, writable: true });
7+
}
8+
9+
afterEach(() => {
10+
Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true });
11+
});
12+
13+
describe('env-path', () => {
14+
let getPathEnvKey: (env: Record<string, string | undefined>) => string;
15+
let getPathEnvValue: (env: Record<string, string | undefined>) => string;
16+
let setPathEnvValue: (
17+
env: Record<string, string | undefined>,
18+
nextPath: string,
19+
) => Record<string, string | undefined>;
20+
let prependPathEntry: (
21+
env: Record<string, string | undefined>,
22+
entry: string,
23+
) => { env: Record<string, string | undefined>; path: string };
24+
25+
beforeEach(async () => {
26+
const mod = await import('@electron/utils/env-path');
27+
getPathEnvKey = mod.getPathEnvKey;
28+
getPathEnvValue = mod.getPathEnvValue;
29+
setPathEnvValue = mod.setPathEnvValue;
30+
prependPathEntry = mod.prependPathEntry;
31+
});
32+
33+
it('prefers Path key on Windows', () => {
34+
setPlatform('win32');
35+
expect(getPathEnvKey({ Path: 'C:\\Windows', PATH: 'C:\\Temp' })).toBe('Path');
36+
});
37+
38+
it('reads path value from Path key on Windows', () => {
39+
setPlatform('win32');
40+
expect(getPathEnvValue({ Path: 'C:\\Windows;C:\\Tools' })).toBe('C:\\Windows;C:\\Tools');
41+
});
42+
43+
it('uses PATH key on non-Windows', () => {
44+
setPlatform('linux');
45+
expect(getPathEnvKey({ PATH: '/usr/bin', Path: '/tmp/bin' })).toBe('PATH');
46+
});
47+
48+
it('removes duplicate path keys when setting a new value', () => {
49+
setPlatform('win32');
50+
const next = setPathEnvValue(
51+
{ Path: 'C:\\A', PATH: 'C:\\B', PaTh: 'C:\\C', HOME: 'C:\\Users\\me' },
52+
'C:\\A;C:\\B',
53+
);
54+
expect(next.Path).toBe('C:\\A;C:\\B');
55+
expect(next.PATH).toBeUndefined();
56+
expect(next.PaTh).toBeUndefined();
57+
expect(next.HOME).toBe('C:\\Users\\me');
58+
});
59+
60+
it('prepends entry with Windows delimiter', () => {
61+
setPlatform('win32');
62+
const next = prependPathEntry({ Path: 'C:\\Windows\\System32' }, 'D:\\clawx\\resources\\bin');
63+
expect(next.path).toBe('D:\\clawx\\resources\\bin;C:\\Windows\\System32');
64+
expect(next.env.Path).toBe('D:\\clawx\\resources\\bin;C:\\Windows\\System32');
65+
});
66+
67+
it('prepends entry with POSIX delimiter', () => {
68+
setPlatform('linux');
69+
const next = prependPathEntry({ PATH: '/usr/bin:/bin' }, '/opt/clawx/bin');
70+
expect(next.path).toBe('/opt/clawx/bin:/usr/bin:/bin');
71+
expect(next.env.PATH).toBe('/opt/clawx/bin:/usr/bin:/bin');
72+
});
73+
});

0 commit comments

Comments
 (0)