Skip to content

Conversation

@ThanhNguyxn
Copy link

Summary

Fixes #44071 - Auto-update creates parallel PowerToys installations.

PR Checklist

Problem

get_current_install_scope() had flawed logic - if HKLM open fails, it checks HKCU. If HKCU has stale perUser value, dowloads wrong installer causing parallel installs.

Solution

  1. Check HKLM for perMachine first
  2. Check HKCU for perUser only if HKLM empty
  3. Fallback to file path detection (LocalAppData = per-user)

Fixes microsoft#44071

The previous logic in get_current_install_scope() had a flaw where if
HKLM registry key open failed (due to permissions or corruption), it
would fall back to checking HKCU. If HKCU had a stale 'perUser' value,
the updater would download the per-user installer for a per-machine
installation, causing parallel installations.

Changes:
- Check HKLM for explicit 'perMachine' InstallScope value first
- Only check HKCU for 'perUser' if HKLM doesn't have the value
- Add fallback detection based on executable path (LocalAppData = per-user)
- Proper resource cleanup with RegCloseKey in all code paths
Copilot AI review requested due to automatic review settings December 6, 2025 03:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves install scope detection logic to prevent parallel PowerToys installations caused by stale registry values. The fix changes the detection order to check HKLM for per-machine installs first, then HKCU for per-user installs, and finally falls back to file path analysis when registry keys are missing or ambiguous. This addresses issue #44071 where auto-update could download the wrong installer type.

Key Changes:

  • Restructured registry check order: HKLM → HKCU → file path fallback
  • Added explicit per-machine check in HKLM before per-user check in HKCU
  • Introduced file path analysis as final fallback to detect LocalAppData installations

return InstallScope::PerMachine;
std::wstring data;
data.resize(dataSize / sizeof(wchar_t));
if (RegGetValueW(hklmKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, &data[0], &dataSize) == ERROR_SUCCESS)
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Use data.data() instead of &data[0] for better code modernization and consistency. While &data[0] works, data.data() is the preferred C++11+ approach for accessing the underlying buffer of a string.

if (RegGetValueW(hklmKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, data.data(), &dataSize) == ERROR_SUCCESS)

Copilot uses AI. Check for mistakes.
nullptr,
&data[0],
&dataSize) != ERROR_SUCCESS)
if (RegGetValueW(hkcuKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, &data[0], &dataSize) == ERROR_SUCCESS)
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Use data.data() instead of &data[0] for better code modernization and consistency, same as the HKLM case.

if (RegGetValueW(hkcuKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, data.data(), &dataSize) == ERROR_SUCCESS)
Suggested change
if (RegGetValueW(hkcuKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, &data[0], &dataSize) == ERROR_SUCCESS)
if (RegGetValueW(hkcuKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, data.data(), &dataSize) == ERROR_SUCCESS)

Copilot uses AI. Check for mistakes.
Comment on lines +93 to +95
if (path.find(L"\\appdata\\local\\") != std::wstring::npos)
{
return InstallScope::PerUser;
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete path check: The fallback logic only checks for \appdata\local\ but doesn't verify that it's actually within the user's profile. A malicious or unusual installation could have a path like C:\SomePath\appdata\local\PowerToys which would incorrectly match as PerUser.

Additionally, the default Program Files paths can vary (e.g., Program Files (x86), non-English Windows with localized folder names, or custom installation drives).

Consider checking against known environment variables or using stricter pattern matching:

wchar_t* localAppData = nullptr;
if (_wdupenv_s(&localAppData, nullptr, L"LOCALAPPDATA") == 0 && localAppData != nullptr)
{
    std::wstring localAppDataPath(localAppData);
    std::transform(localAppDataPath.begin(), localAppDataPath.end(), localAppDataPath.begin(), ::towlower);
    free(localAppData);
    if (path.find(localAppDataPath) == 0)
    {
        return InstallScope::PerUser;
    }
}

This ensures the path actually starts with the user's LocalAppData folder.

Suggested change
if (path.find(L"\\appdata\\local\\") != std::wstring::npos)
{
return InstallScope::PerUser;
wchar_t* localAppData = nullptr;
if (_wdupenv_s(&localAppData, nullptr, L"LOCALAPPDATA") == 0 && localAppData != nullptr)
{
std::wstring localAppDataPath(localAppData);
std::transform(localAppDataPath.begin(), localAppDataPath.end(), localAppDataPath.begin(), ::towlower);
free(localAppData);
// Ensure trailing backslash for proper prefix match
if (!localAppDataPath.empty() && localAppDataPath.back() != L'\\')
{
localAppDataPath += L'\\';
}
if (path.find(localAppDataPath) == 0)
{
return InstallScope::PerUser;
}

Copilot uses AI. Check for mistakes.
if (GetModuleFileNameW(nullptr, modulePath, MAX_PATH) > 0)
{
std::wstring path(modulePath);
std::transform(path.begin(), path.end(), path.begin(), ::towlower);
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing include: std::transform is used on line 92, but <algorithm> is not included in this header file. This will cause a compilation error.

Add the required include at the top of the file:

#include <algorithm>

Copilot uses AI. Check for mistakes.
Comment on lines +32 to +57
HKEY hklmKey{};
if (RegOpenKeyExW(HKEY_LOCAL_MACHINE, INSTALL_SCOPE_REG_KEY, 0, KEY_READ, &hklmKey) == ERROR_SUCCESS)
{
// Open HKCU key
HKEY perUserKey{};
if (RegOpenKeyExW(HKEY_CURRENT_USER,
INSTALL_SCOPE_REG_KEY,
0,
KEY_READ,
&perUserKey) != ERROR_SUCCESS)
DWORD dataSize{};
if (RegGetValueW(hklmKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, nullptr, &dataSize) == ERROR_SUCCESS)
{
// both keys are missing
return InstallScope::PerMachine;
std::wstring data;
data.resize(dataSize / sizeof(wchar_t));
if (RegGetValueW(hklmKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, &data[0], &dataSize) == ERROR_SUCCESS)
{
RegCloseKey(hklmKey);
if (data.find(L"perMachine") != std::wstring::npos)
{
return InstallScope::PerMachine;
}
}
else
{
RegCloseKey(hklmKey);
}
}
else
{
DWORD dataSize{};
if (RegGetValueW(
perUserKey,
nullptr,
L"InstallScope",
RRF_RT_REG_SZ,
nullptr,
nullptr,
&dataSize) != ERROR_SUCCESS)
{
// HKCU key is missing
RegCloseKey(perUserKey);
return InstallScope::PerMachine;
}
RegCloseKey(hklmKey);
}
}
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The codebase uses a RAII pattern with detail::on_exit for managing registry keys (see lines 187, 231, 261 in this file). This approach is more maintainable and exception-safe than manually closing keys at multiple exit points.

Consider refactoring to use this pattern:

HKEY hklmKey{};
if (RegOpenKeyExW(HKEY_LOCAL_MACHINE, INSTALL_SCOPE_REG_KEY, 0, KEY_READ, &hklmKey) == ERROR_SUCCESS)
{
    detail::on_exit closeKey{ [hklmKey] { RegCloseKey(hklmKey); } };
    
    DWORD dataSize{};
    if (RegGetValueW(hklmKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, nullptr, &dataSize) == ERROR_SUCCESS)
    {
        std::wstring data;
        data.resize(dataSize / sizeof(wchar_t));
        if (RegGetValueW(hklmKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, data.data(), &dataSize) == ERROR_SUCCESS)
        {
            if (data.find(L"perMachine") != std::wstring::npos)
            {
                return InstallScope::PerMachine;
            }
        }
    }
}

This eliminates the need for multiple explicit RegCloseKey calls and prevents resource leaks.

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +85
HKEY hkcuKey{};
if (RegOpenKeyExW(HKEY_CURRENT_USER, INSTALL_SCOPE_REG_KEY, 0, KEY_READ, &hkcuKey) == ERROR_SUCCESS)
{
DWORD dataSize{};
if (RegGetValueW(hkcuKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, nullptr, &dataSize) == ERROR_SUCCESS)
{
std::wstring data;
data.resize(dataSize / sizeof(wchar_t));

if (RegGetValueW(
perUserKey,
nullptr,
L"InstallScope",
RRF_RT_REG_SZ,
nullptr,
&data[0],
&dataSize) != ERROR_SUCCESS)
if (RegGetValueW(hkcuKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, &data[0], &dataSize) == ERROR_SUCCESS)
{
// HKCU key is missing
RegCloseKey(perUserKey);
return InstallScope::PerMachine;
RegCloseKey(hkcuKey);
if (data.find(L"perUser") != std::wstring::npos)
{
return InstallScope::PerUser;
}
}
RegCloseKey(perUserKey);

if (data.contains(L"perUser"))
else
{
return InstallScope::PerUser;
RegCloseKey(hkcuKey);
}
}
else
{
RegCloseKey(hkcuKey);
}
}
Copy link

Copilot AI Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the HKLM case - consider using the detail::on_exit RAII pattern for managing the registry key, which is the established pattern in this codebase:

HKEY hkcuKey{};
if (RegOpenKeyExW(HKEY_CURRENT_USER, INSTALL_SCOPE_REG_KEY, 0, KEY_READ, &hkcuKey) == ERROR_SUCCESS)
{
    detail::on_exit closeKey{ [hkcuKey] { RegCloseKey(hkcuKey); } };
    
    DWORD dataSize{};
    if (RegGetValueW(hkcuKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, nullptr, &dataSize) == ERROR_SUCCESS)
    {
        std::wstring data;
        data.resize(dataSize / sizeof(wchar_t));
        if (RegGetValueW(hkcuKey, nullptr, L"InstallScope", RRF_RT_REG_SZ, nullptr, data.data(), &dataSize) == ERROR_SUCCESS)
        {
            if (data.find(L"perUser") != std::wstring::npos)
            {
                return InstallScope::PerUser;
            }
        }
    }
}

Copilot uses AI. Check for mistakes.
@github-actions

This comment has been minimized.

@ThanhNguyxn ThanhNguyxn force-pushed the fix/auto-update-parallel-install branch from 81c3fd4 to 1754d8d Compare December 6, 2025 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto update not working

1 participant