Skip to content

fix: Improve install scope detection to prevent parallel installations #44112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ThanhNguyxn wants to merge 2 commits into from
+59 −50
Closed

fix: Improve install scope detection to prevent parallel installations #44112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
efa1103
fix: Improve install scope detection to prevent parallel installations
ThanhNguyxn Dec 6, 2025
1754d8d
chore: Add hkcu and hklm to spell-check expect list
ThanhNguyxn Dec 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .github/actions/spell-check/expect.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@
ENABLEDPOPUP
ENABLETAB
ENABLETEMPLATE
encodedlaunch

Check warning

Code scanning / check-spelling

Ignored Expect Variant Warning

ekus is ignored by check-spelling because another more general variant is also in expect. (ignored-expect-variant)
encryptor
ENDSESSION
ENSUREVISIBLE
Expand Down Expand Up @@ -674,7 +674,9 @@
HKCOMB
HKCR
HKCU
hkcu
hkey
hklm
HKLM
HKM
hkmng
Expand Down Expand Up @@ -1057,9 +1059,9 @@
Mso
msrc
msstore
mstsc
msvcp
MT
mstsc
MTND
MULTIPLEUSE
multizone
Expand Down Expand Up @@ -1203,17 +1205,17 @@
oleaut
OLECHAR
ollama
ollama
onebranch
onnx
onnx
OOBEUI
openas
opencode
OPENFILENAME
openrdp
opensource
openxmlformats
ollama
onnx
OPTIMIZEFORINVOKE
ORPHANEDDIALOGTITLE
ORSCANS
Expand Down Expand Up @@ -1426,7 +1428,6 @@
rbhid
rclsid
RCZOOMIT
remotedesktop
rdp
RDW
READMODE
Expand Down Expand Up @@ -1455,6 +1456,7 @@
REMAPSUCCESSFUL
REMAPUNSUCCESSFUL
Remotable
remotedesktop
remoteip
Removelnk
renamable
Expand Down
99 changes: 53 additions & 46 deletions src/common/utils/registry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,65 +28,72 @@ namespace registry

inline const InstallScope get_current_install_scope()
{
// Open HKLM key
HKEY perMachineKey{};
if (RegOpenKeyExW(HKEY_LOCAL_MACHINE,
INSTALL_SCOPE_REG_KEY,
0,
KEY_READ,
&perMachineKey) != ERROR_SUCCESS)
// First, check HKLM for explicit per-machine install scope
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)
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.
{
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);
}
}
Comment on lines +32 to +57
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.

// Then check HKCU for per-user install
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)
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.
{
// 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);
}
}
Comment on lines +60 to +85
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.

// Fallback: Check if running from LocalAppData (per-user) vs Program Files (per-machine)
wchar_t modulePath[MAX_PATH];
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.
if (path.find(L"\\appdata\\local\\") != std::wstring::npos)
{
return InstallScope::PerUser;
Comment on lines +93 to +95
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.
}
}

return InstallScope::PerMachine;
Expand Down