From 45fe53fa4aa6dd1e8ae74bfe926ec7170de9c608 Mon Sep 17 00:00:00 2001 From: Amishaben Ramani <130687844+amishabenramani@users.noreply.github.com> Date: Fri, 25 Sep 2026 17:19:42 +0200 Subject: [PATCH] fix(windows): repair legacy environment variable types --- .../path-extender-windows.spec.ts | 2 +- .../path-extender-windows.ts | 21 +++--- .../registry-type.spec.ts | 67 +++++++++++++++++++ 3 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 os/env/path-extender-windows/registry-type.spec.ts diff --git a/os/env/path-extender-windows/path-extender-windows.spec.ts b/os/env/path-extender-windows/path-extender-windows.spec.ts index 0d9763f..3c4d0bd 100644 --- a/os/env/path-extender-windows/path-extender-windows.spec.ts +++ b/os/env/path-extender-windows/path-extender-windows.spec.ts @@ -306,7 +306,7 @@ test('PNPM_HOME is already set, but Path is updated', async () => { failed: false, stdout: ` HKEY_CURRENT_USER\\Environment - PNPM_HOME REG_EXPAND_SZ ${pnpmHomeDirNormalized} + PNPM_HOME REG_SZ ${pnpmHomeDirNormalized} Path REG_EXPAND_SZ ${currentPathInRegistry} `, }).mockResolvedValueOnce({ diff --git a/os/env/path-extender-windows/path-extender-windows.ts b/os/env/path-extender-windows/path-extender-windows.ts index af761bd..1a4ab0d 100644 --- a/os/env/path-extender-windows/path-extender-windows.ts +++ b/os/env/path-extender-windows/path-extender-windows.ts @@ -90,21 +90,26 @@ async function updateEnvVariable ( overwrite: boolean } ): Promise { - const currentValue = await getEnvValueFromRegistry(registryOutput, name) + const current = await getEnvVariableFromRegistry(registryOutput, name) + const currentValue = current?.data if (currentValue && !opts.overwrite) { if (currentValue !== value) { throw new BadEnvVariableError({ envName: name, currentValue, wantedValue: value }) } - return { variable: name, action: 'skipped', oldValue: currentValue, newValue: value } - } else { - await setEnvVarInRegistry(name, value, { expandableString: opts.expandableString }) - return { variable: name, action: 'updated', oldValue: currentValue as string, newValue: value } + const wantedType = opts.expandableString ? 'REG_EXPAND_SZ' : 'REG_SZ' + // Older pnpm versions stored PNPM_HOME as REG_EXPAND_SZ. Repair its type + // even when the path is unchanged, without requiring an overwrite. + if (current?.type === wantedType) { + return { variable: name, action: 'skipped', oldValue: currentValue, newValue: value } + } } + await setEnvVarInRegistry(name, value, { expandableString: opts.expandableString }) + return { variable: name, action: 'updated', oldValue: currentValue, newValue: value } } async function addToPath (registryOutput: string, addedDir: string, position: AddingPosition = 'start'): Promise { const variable = 'Path' - const pathData = await getEnvValueFromRegistry(registryOutput, variable) + const pathData = (await getEnvVariableFromRegistry(registryOutput, variable))?.data if (pathData === undefined || pathData == null || pathData.trim() === '') { throw new PnpmError('NO_PATH', '"Path" environment variable is not found in the registry') } else if (pathData.split(path.delimiter).includes(addedDir)) { @@ -135,10 +140,10 @@ async function getRegistryOutput (): Promise { } } -async function getEnvValueFromRegistry (registryOutput: string, envVarName: string): Promise { +async function getEnvVariableFromRegistry (registryOutput: string, envVarName: string): Promise { const regexp = new RegExp(`^ {4}(?${envVarName}) {4}(?\\w+) {4}(?.*)$`, 'gim') const match = Array.from(matchAll(registryOutput, regexp))[0] as IEnvironmentValueMatch - return match?.groups.data + return match?.groups } async function setEnvVarInRegistry ( diff --git a/os/env/path-extender-windows/registry-type.spec.ts b/os/env/path-extender-windows/registry-type.spec.ts new file mode 100644 index 0000000..f665b89 --- /dev/null +++ b/os/env/path-extender-windows/registry-type.spec.ts @@ -0,0 +1,67 @@ +import execa from 'safe-execa' +import { addDirToWindowsEnvPath } from './path-extender-windows' + +jest.mock('safe-execa') + +const regKey = 'HKEY_CURRENT_USER\\Environment' +const home = 'C:\\Users\\Test User\\AppData\\Local\\pnpm' +const userPath = '%PNPM_HOME%\\bin;%USERPROFILE%\\WindowsApps' +const opts = { proxyVarName: 'PNPM_HOME', proxyVarSubDir: 'bin' } + +function mockRegistry (type: string, value = home) { + execa['mockReset']() + execa['mockResolvedValueOnce']({ failed: false, stdout: 'Active code page: 437' }) + .mockResolvedValueOnce({ failed: false, stdout: '' }) + .mockResolvedValueOnce({ + failed: false, + stdout: `\r\n${regKey}\r\n PNPM_HOME ${type} ${value}\r\n Path REG_EXPAND_SZ ${userPath}\r\n`, + }) + .mockResolvedValue({ failed: false, stdout: '' }) +} + +function registryWrites () { + return execa['mock'].calls.filter(([command, args]: [string, string[]]) => command === 'reg' && args[0] === 'add') +} + +test('repairs legacy PNPM_HOME without duplicating Path, then skips a repeat setup', async () => { + mockRegistry('REG_EXPAND_SZ') + const report = await addDirToWindowsEnvPath(home, opts) + expect(report).toStrictEqual([ + { variable: 'PNPM_HOME', action: 'updated', oldValue: home, newValue: home }, + { variable: 'Path', action: 'skipped', oldValue: userPath, newValue: userPath }, + ]) + expect(registryWrites()).toStrictEqual([ + ['reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_SZ', '/d', home, '/f'], { windowsHide: false }], + ]) + expect(execa).toHaveBeenLastCalledWith('chcp', ['437']) + + mockRegistry('REG_SZ') + const repeatReport = await addDirToWindowsEnvPath(home, opts) + expect(repeatReport.every(change => change.action === 'skipped')).toBe(true) + expect(registryWrites()).toStrictEqual([]) +}) + +test.each(['REG_SZ', 'REG_EXPAND_SZ'])('preserves a different PNPM_HOME stored as %s', async (type) => { + mockRegistry(type, 'C:\\other') + await expect(addDirToWindowsEnvPath(home, opts)).rejects.toMatchObject({ code: 'ERR_PNPM_BAD_ENV_FOUND' }) + expect(registryWrites()).toStrictEqual([]) + expect(execa).toHaveBeenLastCalledWith('chcp', ['437']) +}) + +test.each(['REG_SZ', 'REG_EXPAND_SZ'])('force replaces a different PNPM_HOME stored as %s', async (type) => { + mockRegistry(type, 'C:\\other') + const report = await addDirToWindowsEnvPath(home, { ...opts, overwriteProxyVar: true }) + expect(report[0]).toStrictEqual({ variable: 'PNPM_HOME', action: 'updated', oldValue: 'C:\\other', newValue: home }) + expect(registryWrites()).toStrictEqual([ + ['reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_SZ', '/d', home, '/f'], { windowsHide: false }], + ]) +}) + +test('propagates a failed type repair and restores the code page', async () => { + mockRegistry('REG_EXPAND_SZ') + execa['mockRejectedValueOnce'](Object.assign(new Error('Access denied'), { stderr: 'Access denied' })) + await expect(addDirToWindowsEnvPath(home, opts)).rejects.toMatchObject({ code: 'ERR_PNPM_FAILED_SET_ENV' }) + expect(registryWrites()).toHaveLength(1) + expect(execa['mock'].calls.some(([command]: [string]) => command === 'setx')).toBe(false) + expect(execa).toHaveBeenLastCalledWith('chcp', ['437']) +})