diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index f9afd0fbb5..6e581b1893 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -227,9 +227,6 @@ Source Files\Common - - Source Files - Source Files\Common @@ -242,9 +239,6 @@ Source Files\Repository - - Source Files - Source Files\Common @@ -293,6 +287,12 @@ Source Files\CLI + + Source Files\Common + + + Source Files\Repository + diff --git a/src/AppInstallerCLITests/Filesystem.cpp b/src/AppInstallerCLITests/Filesystem.cpp index f3bff14163..c1d0db7f1e 100644 --- a/src/AppInstallerCLITests/Filesystem.cpp +++ b/src/AppInstallerCLITests/Filesystem.cpp @@ -86,4 +86,18 @@ TEST_CASE("VerifyIsSameVolume", "[filesystem]") REQUIRE_FALSE(IsSameVolume(path5, path6)); REQUIRE_FALSE(IsSameVolume(path4, path6)); REQUIRE_FALSE(IsSameVolume(path6, path8)); -} \ No newline at end of file +} + +TEST_CASE("ReplaceCommonPathPrefix", "[filesystem]") +{ + std::filesystem::path prefix = "C:\\test1\\test2"; + std::string replacement = "%TEST%"; + + std::filesystem::path shouldReplace = "C:\\test1\\test2\\subdir1\\subdir2"; + REQUIRE(ReplaceCommonPathPrefix(shouldReplace, prefix, replacement)); + REQUIRE(shouldReplace.u8string() == (replacement + "\\subdir1\\subdir2")); + + std::filesystem::path shouldNotReplace = "C:\\test1\\test3\\subdir1\\subdir2"; + REQUIRE(!ReplaceCommonPathPrefix(shouldNotReplace, prefix, replacement)); + REQUIRE(shouldNotReplace.u8string() == "C:\\test1\\test3\\subdir1\\subdir2"); +} diff --git a/src/AppInstallerCLITests/Runtime.cpp b/src/AppInstallerCLITests/Runtime.cpp index 4be693db71..65dc959a57 100644 --- a/src/AppInstallerCLITests/Runtime.cpp +++ b/src/AppInstallerCLITests/Runtime.cpp @@ -2,7 +2,9 @@ // Licensed under the MIT License. #include "pch.h" #include "TestCommon.h" +#include "TestHooks.h" #include +#include using namespace AppInstaller; using namespace AppInstaller::Runtime; @@ -113,4 +115,22 @@ TEST_CASE("VerifyDevModeEnabledCheck", "[runtime]") REQUIRE(modifiedState != initialState); REQUIRE(revertedState == initialState); -} \ No newline at end of file +} + +TEST_CASE("EnsureUserProfileNotPresentInDisplayPaths", "[runtime]") +{ + // Clear the overrides that we use when testing as they don't consider display purposes + Runtime::TestHook_ClearPathOverrides(); + auto restorePaths = wil::scope_exit([]() { TestCommon::SetTestPathOverrides(); }); + + std::filesystem::path userProfilePath = Filesystem::GetKnownFolderPath(FOLDERID_Profile); + std::string userProfileString = userProfilePath.u8string(); + + for (auto i = ToIntegral(ToEnum(0)); i < ToIntegral(PathName::Max); ++i) + { + std::filesystem::path displayPath = GetPathTo(ToEnum(i), true); + std::string displayPathString = displayPath.u8string(); + INFO(i << " = " << displayPathString); + REQUIRE(displayPathString.find(userProfileString) == std::string::npos); + } +} diff --git a/src/AppInstallerCLITests/TestCommon.cpp b/src/AppInstallerCLITests/TestCommon.cpp index 9abea67321..bb8d759637 100644 --- a/src/AppInstallerCLITests/TestCommon.cpp +++ b/src/AppInstallerCLITests/TestCommon.cpp @@ -8,6 +8,8 @@ #include #include +using namespace AppInstaller; + namespace TestCommon { namespace @@ -363,4 +365,15 @@ namespace TestCommon return root; } + + void SetTestPathOverrides() + { + // Force all tests to run against settings inside this container. + // This prevents test runs from trashing the users actual settings. + Runtime::TestHook_SetPathOverride(Runtime::PathName::LocalState, Runtime::GetPathTo(Runtime::PathName::LocalState) / "Tests"); + Runtime::TestHook_SetPathOverride(Runtime::PathName::UserFileSettings, Runtime::GetPathTo(Runtime::PathName::UserFileSettings) / "Tests"); + Runtime::TestHook_SetPathOverride(Runtime::PathName::StandardSettings, Runtime::GetPathTo(Runtime::PathName::StandardSettings) / "Tests"); + Runtime::TestHook_SetPathOverride(Runtime::PathName::SecureSettingsForRead, Runtime::GetPathTo(Runtime::PathName::StandardSettings) / "WinGet_SecureSettings_Tests"); + Runtime::TestHook_SetPathOverride(Runtime::PathName::SecureSettingsForWrite, Runtime::GetPathDetailsFor(Runtime::PathName::SecureSettingsForRead)); + } } diff --git a/src/AppInstallerCLITests/TestCommon.h b/src/AppInstallerCLITests/TestCommon.h index c6e719362b..5866068eed 100644 --- a/src/AppInstallerCLITests/TestCommon.h +++ b/src/AppInstallerCLITests/TestCommon.h @@ -153,4 +153,7 @@ namespace TestCommon // Convert to Json::Value Json::Value ConvertToJson(const std::string& content); + + // Sets up the test path overrides. + void SetTestPathOverrides(); } diff --git a/src/AppInstallerCLITests/main.cpp b/src/AppInstallerCLITests/main.cpp index 2ae3396f67..d38da19342 100644 --- a/src/AppInstallerCLITests/main.cpp +++ b/src/AppInstallerCLITests/main.cpp @@ -13,7 +13,6 @@ #include #include "TestCommon.h" -#include "TestHooks.h" #include "TestSettings.h" using namespace winrt; @@ -151,13 +150,7 @@ int main(int argc, char** argv) // Forcibly enable event writing to catch bugs in that code g_IsTelemetryProviderEnabled = true; - // Force all tests to run against settings inside this container. - // This prevents test runs from trashing the users actual settings. - Runtime::TestHook_SetPathOverride(Runtime::PathName::LocalState, Runtime::GetPathTo(Runtime::PathName::LocalState) / "Tests"); - Runtime::TestHook_SetPathOverride(Runtime::PathName::UserFileSettings, Runtime::GetPathTo(Runtime::PathName::UserFileSettings) / "Tests"); - Runtime::TestHook_SetPathOverride(Runtime::PathName::StandardSettings, Runtime::GetPathTo(Runtime::PathName::StandardSettings) / "Tests"); - Runtime::TestHook_SetPathOverride(Runtime::PathName::SecureSettingsForRead, Runtime::GetPathTo(Runtime::PathName::StandardSettings) / "WinGet_SecureSettings_Tests"); - Runtime::TestHook_SetPathOverride(Runtime::PathName::SecureSettingsForWrite, Runtime::GetPathDetailsFor(Runtime::PathName::SecureSettingsForRead)); + TestCommon::SetTestPathOverrides(); // Remove any existing settings files in the new tests path TestCommon::UserSettingsFileGuard settingsGuard; diff --git a/src/AppInstallerCLITests/pch.h b/src/AppInstallerCLITests/pch.h index 328fa5c20f..fdceff3fea 100644 --- a/src/AppInstallerCLITests/pch.h +++ b/src/AppInstallerCLITests/pch.h @@ -9,6 +9,7 @@ #include #include #include +#include #include diff --git a/src/AppInstallerCommonCore/Filesystem.cpp b/src/AppInstallerCommonCore/Filesystem.cpp index e194710832..732cb0a310 100644 --- a/src/AppInstallerCommonCore/Filesystem.cpp +++ b/src/AppInstallerCommonCore/Filesystem.cpp @@ -209,7 +209,7 @@ namespace AppInstaller::Filesystem while (prefixItr != prefix.end() && sourceItr != source.end()) { - if (Utility::ICUCaseInsensitiveEquals(prefixItr->u8string(), sourceItr->u8string())) + if (!Utility::ICUCaseInsensitiveEquals(prefixItr->u8string(), sourceItr->u8string())) { break; } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h index ebf3caca45..e14fd2772d 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h @@ -47,6 +47,8 @@ namespace AppInstaller::Runtime PortableLinksMachineLocation, // The root location for the package containing the winget application. SelfPackageRoot, + // Always one more than the last path; for being able to iterate paths in tests. + Max }; // The principal that an ACE applies to. diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp index 040185c6ba..f6e4a805e9 100644 --- a/src/AppInstallerCommonCore/Runtime.cpp +++ b/src/AppInstallerCommonCore/Runtime.cpp @@ -38,6 +38,9 @@ namespace AppInstaller::Runtime #endif constexpr std::string_view s_RuntimePath_Unpackaged_DefaultState = "defaultState"sv; + constexpr std::string_view s_UserProfileEnvironmentVariable = "%USERPROFILE%"; + constexpr std::string_view s_LocalAppDataEnvironmentVariable = "%LOCALAPPDATA%"; + static std::optional s_runtimePathStateName; static wil::srwlock s_runtimePathStateNameLock; @@ -108,35 +111,42 @@ namespace AppInstaller::Runtime #endif // Gets the user's temp path - std::filesystem::path GetPathToUserTemp() + std::filesystem::path GetPathToUserTemp(bool forDisplay) { - wchar_t tempPath[MAX_PATH + 1]; - DWORD tempChars = GetTempPathW(ARRAYSIZE(tempPath), tempPath); - THROW_LAST_ERROR_IF(!tempChars); - THROW_HR_IF(E_UNEXPECTED, tempChars > ARRAYSIZE(tempPath)); - return { std::wstring_view{ tempPath, static_cast(tempChars) } }; + if (forDisplay) + { + return "%TEMP%"; + } + else + { + wchar_t tempPath[MAX_PATH + 1]; + DWORD tempChars = GetTempPathW(ARRAYSIZE(tempPath), tempPath); + THROW_LAST_ERROR_IF(!tempChars); + THROW_HR_IF(E_UNEXPECTED, tempChars > ARRAYSIZE(tempPath)); + return { std::wstring_view{ tempPath, static_cast(tempChars) } }; + } } // Gets the path to the appdata root. // *Only used by non packaged version!* - std::filesystem::path GetPathToAppDataRoot() + std::filesystem::path GetPathToAppDataRoot(bool forDisplay) { THROW_HR_IF(E_NOT_VALID_STATE, IsRunningInPackagedContext()); - std::filesystem::path result = GetKnownFolderPath(FOLDERID_LocalAppData); + std::filesystem::path result = forDisplay ? s_LocalAppDataEnvironmentVariable : GetKnownFolderPath(FOLDERID_LocalAppData); result /= "Microsoft/WinGet"; return result; } // Gets the path to the app data relative directory. - std::filesystem::path GetPathToAppDataDir(const std::filesystem::path& relative) + std::filesystem::path GetPathToAppDataDir(const std::filesystem::path& relative, bool forDisplay) { THROW_HR_IF(E_INVALIDARG, !relative.has_relative_path()); THROW_HR_IF(E_INVALIDARG, relative.has_root_path()); THROW_HR_IF(E_INVALIDARG, !relative.has_filename()); - std::filesystem::path result = GetPathToAppDataRoot(); + std::filesystem::path result = GetPathToAppDataRoot(forDisplay); result /= relative; return result; @@ -206,6 +216,15 @@ namespace AppInstaller::Runtime PSID SID; TRUSTEE_TYPE TrusteeType; }; + + // Try to replace LOCALAPPDATA first as it is the likely location, fall back to trying USERPROFILE. + void ReplaceProfilePathsWithEnvironmentVariable(std::filesystem::path& path) + { + if (!ReplaceCommonPathPrefix(path, GetKnownFolderPath(FOLDERID_LocalAppData), s_LocalAppDataEnvironmentVariable)) + { + ReplaceCommonPathPrefix(path, GetKnownFolderPath(FOLDERID_Profile), s_UserProfileEnvironmentVariable); + } + } } void SetRuntimePathStateName(std::string name) @@ -308,14 +327,18 @@ namespace AppInstaller::Runtime } // Contains all of the paths that are common between the runtime contexts. - PathDetails GetPathDetailsCommon(PathName path) + PathDetails GetPathDetailsCommon(PathName path, bool forDisplay) { PathDetails result; + // We should not create directories by default when they are retrieved for display purposes. + result.Create = !forDisplay; + + bool mayBeInProfilePath = false; switch (path) { case PathName::UserProfile: - result.Path = GetKnownFolderPath(FOLDERID_Profile); + result.Path = forDisplay ? s_UserProfileEnvironmentVariable : GetKnownFolderPath(FOLDERID_Profile); result.Create = false; break; case PathName::PortablePackageUserRoot: @@ -327,6 +350,7 @@ namespace AppInstaller::Runtime result.Path /= s_PortablePackageRoot; result.Path /= s_PortablePackagesDirectory; } + mayBeInProfilePath = true; break; case PathName::PortablePackageMachineRoot: result.Path = Settings::User().Get(); @@ -351,6 +375,7 @@ namespace AppInstaller::Runtime result.Path /= s_PortablePackageUserRoot_Base; result.Path /= s_PortablePackageRoot; result.Path /= s_LinksDirectory; + mayBeInProfilePath = true; break; case PathName::PortableLinksMachineLocation: result.Path = GetKnownFolderPath(FOLDERID_ProgramFiles); @@ -361,20 +386,28 @@ namespace AppInstaller::Runtime THROW_HR(E_UNEXPECTED); } + if (mayBeInProfilePath && forDisplay) + { + ReplaceProfilePathsWithEnvironmentVariable(result.Path); + } + return result; } #ifndef WINGET_DISABLE_FOR_FUZZING - PathDetails GetPathDetailsForPackagedContext(PathName path, bool forDisplay = false) + PathDetails GetPathDetailsForPackagedContext(PathName path, bool forDisplay) { PathDetails result; + // We should not create directories by default when they are retrieved for display purposes. + result.Create = !forDisplay; auto appStorage = winrt::Windows::Storage::ApplicationData::Current(); + bool mayBeInProfilePath = false; switch (path) { case PathName::Temp: - result.Path = GetPathToUserTemp() / s_DefaultTempDirectory; + result.Path = GetPathToUserTemp(forDisplay) / s_DefaultTempDirectory; result.SetOwner(ACEPrincipal::CurrentUser); result.ACL[ACEPrincipal::System] = ACEPermissions::All; result.ACL[ACEPrincipal::Admins] = ACEPermissions::All; @@ -382,15 +415,13 @@ namespace AppInstaller::Runtime case PathName::LocalState: case PathName::UserFileSettings: result.Path.assign(appStorage.LocalFolder().Path().c_str()); + mayBeInProfilePath = true; break; case PathName::DefaultLogLocation: // To enable UIF collection through Feedback hub, we must put our logs here. result.Path.assign(appStorage.LocalFolder().Path().c_str()); result.Path /= WINGET_DEFAULT_LOG_DIRECTORY; - if (forDisplay) - { - ReplaceCommonPathPrefix(result.Path, GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%"); - } + mayBeInProfilePath = true; break; case PathName::StandardSettings: result.Create = false; @@ -417,15 +448,11 @@ namespace AppInstaller::Runtime case PathName::PortablePackageMachineRoot: case PathName::PortablePackageMachineRootX86: case PathName::PortableLinksMachineLocation: - result = GetPathDetailsCommon(path); + result = GetPathDetailsCommon(path, forDisplay); break; case PathName::PortableLinksUserLocation: case PathName::PortablePackageUserRoot: - result = GetPathDetailsCommon(path); - if (forDisplay) - { - ReplaceCommonPathPrefix(result.Path, GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%"); - } + result = GetPathDetailsCommon(path, forDisplay); break; case PathName::SelfPackageRoot: result.Path = GetPackagePath(); @@ -435,29 +462,27 @@ namespace AppInstaller::Runtime THROW_HR(E_UNEXPECTED); } + if (mayBeInProfilePath && forDisplay) + { + ReplaceProfilePathsWithEnvironmentVariable(result.Path); + } + return result; } #endif - PathDetails GetPathDetailsForUnpackagedContext(PathName path, bool forDisplay = false) + PathDetails GetPathDetailsForUnpackagedContext(PathName path, bool forDisplay) { PathDetails result; + // We should not create directories by default when they are retrieved for display purposes. + result.Create = !forDisplay; switch (path) { case PathName::Temp: case PathName::DefaultLogLocation: { - if (forDisplay) - { - result.Path.assign("%TEMP%"); - result.Create = false; - } - else - { - result.Path = GetPathToUserTemp(); - } - + result.Path = GetPathToUserTemp(forDisplay); result.Path /= s_DefaultTempDirectory; result.Path /= GetRuntimePathStateName(); if (path == PathName::Temp) @@ -469,13 +494,13 @@ namespace AppInstaller::Runtime } break; case PathName::LocalState: - result.Path = GetPathToAppDataDir(s_AppDataDir_State); + result.Path = GetPathToAppDataDir(s_AppDataDir_State, forDisplay); result.Path /= GetRuntimePathStateName(); result.SetOwner(ACEPrincipal::CurrentUser); break; case PathName::StandardSettings: case PathName::UserFileSettings: - result.Path = GetPathToAppDataDir(s_AppDataDir_Settings); + result.Path = GetPathToAppDataDir(s_AppDataDir_Settings, forDisplay); result.Path /= GetRuntimePathStateName(); result.SetOwner(ACEPrincipal::CurrentUser); break; @@ -501,15 +526,11 @@ namespace AppInstaller::Runtime case PathName::PortablePackageMachineRoot: case PathName::PortablePackageMachineRootX86: case PathName::PortableLinksMachineLocation: - result = GetPathDetailsCommon(path); + result = GetPathDetailsCommon(path, forDisplay); break; case PathName::PortableLinksUserLocation: case PathName::PortablePackageUserRoot: - result = GetPathDetailsCommon(path); - if (forDisplay) - { - ReplaceCommonPathPrefix(result.Path, GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%"); - } + result = GetPathDetailsCommon(path, forDisplay); break; case PathName::SelfPackageRoot: result.Path = GetBinaryDirectoryPath(); @@ -534,7 +555,7 @@ namespace AppInstaller::Runtime else #endif { - result = GetPathDetailsForUnpackagedContext(path); + result = GetPathDetailsForUnpackagedContext(path, forDisplay); } #ifndef AICLI_DISABLE_TEST_HOOKS