fix: make the proxy var non-expandable

ref pnpm/pnpm#4658 (comment)
pnpm · Nov 21, 2022 · 4d8f283 · 4d8f283
1 parent f2f248e
commit 4d8f283
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 8 deletions.
diff --git a/os/env/path-extender-windows/path-extender-windows.spec.ts b/os/env/path-extender-windows/path-extender-windows.spec.ts
@@ -142,7 +142,7 @@ HKEY_CURRENT_USER\\Environment
     },
   ])
   expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey], { windowsHide: false })
-  expect(execa).toHaveBeenNthCalledWith(4, 'reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_EXPAND_SZ', '/d', pnpmHomeDirNormalized, '/f'], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(4, 'reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_SZ', '/d', pnpmHomeDirNormalized, '/f'], { windowsHide: false })
   expect(execa).toHaveBeenNthCalledWith(5, 'reg', ['add', regKey, '/v', 'Path', '/t', 'REG_EXPAND_SZ', '/d', `%PNPM_HOME%;${currentPathInRegistry}`, '/f'], { windowsHide: false })
   expect(execa).toHaveBeenNthCalledWith(6, 'setx', ['REFRESH_ENV_VARS', '1'], { windowsHide: false })
   expect(execa).toHaveBeenNthCalledWith(7, 'reg', ['delete' ,regKey, '/v', 'REFRESH_ENV_VARS', '/f'], { windowsHide: false })
@@ -372,7 +372,7 @@ HKEY_CURRENT_USER\\Environment
     },
   ])
   expect(execa).toHaveBeenNthCalledWith(3, 'reg', ['query', regKey], { windowsHide: false })
-  expect(execa).toHaveBeenNthCalledWith(4, 'reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_EXPAND_SZ', '/d', pnpmHomeDirNormalized, '/f'], { windowsHide: false })
+  expect(execa).toHaveBeenNthCalledWith(4, 'reg', ['add', regKey, '/v', 'PNPM_HOME', '/t', 'REG_SZ', '/d', pnpmHomeDirNormalized, '/f'], { windowsHide: false })
 })
 
 test('failure to install', async () => {

diff --git a/os/env/path-extender-windows/path-extender-windows.ts b/os/env/path-extender-windows/path-extender-windows.ts
@@ -62,7 +62,10 @@ async function _addDirToWindowsEnvPath (dir: string, opts: AddDirToWindowsEnvPat
   const registryOutput = await getRegistryOutput()
   const changes: PathExtenderWindowsReport = []
   if (opts.proxyVarName) {
-    changes.push(await updateEnvVariable(registryOutput, opts.proxyVarName, addedDir, { overwrite: opts.overwriteProxyVar }))
+    changes.push(await updateEnvVariable(registryOutput, opts.proxyVarName, addedDir, {
+      expandableString: false,
+      overwrite: opts.overwriteProxyVar,
+    }))
     changes.push(await addToPath(registryOutput, `%${opts.proxyVarName}%`, opts.position))
   } else {
     changes.push(await addToPath(registryOutput, addedDir, opts.position))
@@ -71,15 +74,23 @@ async function _addDirToWindowsEnvPath (dir: string, opts: AddDirToWindowsEnvPat
   return changes
 }
 
-async function updateEnvVariable (registryOutput: string, name: string, value: string, opts: { overwrite: boolean }): Promise<EnvVariableChange> {
+async function updateEnvVariable (
+  registryOutput: string,
+  name: string,
+  value: string,
+  opts: {
+    expandableString: boolean
+    overwrite: boolean
+  }
+): Promise<EnvVariableChange> {
   const currentValue = await getEnvValueFromRegistry(registryOutput, name)
   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)
+    await setEnvVarInRegistry(name, value, { expandableString: opts.expandableString })
     return { variable: name, action: 'updated', oldValue: currentValue, newValue: value }
   }
 }
@@ -95,7 +106,7 @@ async function addToPath (registryOutput: string, addedDir: string, position: Ad
     const newPathValue = position === 'start'
       ? `${addedDir}${path.delimiter}${pathData}`
       : `${pathData}${path.delimiter}${addedDir}`
-    await setEnvVarInRegistry('Path', newPathValue)
+    await setEnvVarInRegistry('Path', newPathValue, { expandableString: true })
     return { action: 'updated', variable, oldValue: pathData, newValue: newPathValue }
   }
 }
@@ -123,9 +134,16 @@ async function getEnvValueFromRegistry (registryOutput: string, envVarName: stri
   return match?.groups.data
 }
 
-async function setEnvVarInRegistry (envVarName: string, envVarValue: string) {
+async function setEnvVarInRegistry (
+  envVarName: string,
+  envVarValue: string,
+  opts: {
+    expandableString: boolean
+  }
+) {
+  const regType = opts.expandableString ? 'REG_EXPAND_SZ' : 'REG_SZ'
   try {
-    await execa('reg', ['add', REG_KEY, '/v', envVarName, '/t', 'REG_EXPAND_SZ', '/d', envVarValue, '/f'], EXEC_OPTS)
+    await execa('reg', ['add', REG_KEY, '/v', envVarName, '/t', regType, '/d', envVarValue, '/f'], EXEC_OPTS)
   } catch (err: any) { // eslint-disable-line
     throw new PnpmError('FAILED_SET_ENV', `Failed to set "${envVarName}" to "${envVarValue}": ${err.stderr as string}`)
   }