commit 3d7f35cc274fa4a744360018de5c28e84760dfbf
parent 27a1b61c3c30091274ed55f7fb37b0fce4eae358
Author: Ryan Fu <69221034+ryfu-msft@users.noreply.github.com>
Date: Wed, 27 Jul 2022 09:00:07 -0700
Check for symlink creation privilege for portable install (#2369)
Diffstat:
7 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp
@@ -2,6 +2,7 @@
// Licensed under the MIT License.
#include "pch.h"
#include "PortableFlow.h"
+#include "WorkflowBase.h"
#include "winget/Filesystem.h"
#include "winget/PortableARPEntry.h"
#include "AppInstallerStrings.h"
@@ -540,6 +541,21 @@ namespace AppInstaller::CLI::Workflow
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_PORTABLE_REPARSE_POINT_NOT_SUPPORTED);
}
}
+
+ // TODO: Remove entire task once issue regarding symlink creation privilege has been resolved.
+ void EnsureSymlinkCreationPrivilege(Execution::Context& context)
+ {
+ if (!Runtime::IsDevModeEnabled())
+ {
+ AICLI_LOG(CLI, Info, << "Developer mode not enabled.");
+ context << Workflow::EnsureRunningAsAdmin;
+
+ if (context.IsTerminated())
+ {
+ context.Reporter.Error() << std::endl << "https://github.com/microsoft/winget-cli/issues/2368" << std::endl;
+ }
+ }
+ }
}
void PortableInstallImpl(Execution::Context& context)
@@ -617,6 +633,7 @@ namespace AppInstaller::CLI::Workflow
if (installerType == InstallerTypeEnum::Portable)
{
context <<
+ EnsureSymlinkCreationPrivilege <<
EnsureValidArgsForPortableInstall <<
EnsureVolumeSupportsReparsePoints;
}
diff --git a/src/AppInstallerCLITests/Runtime.cpp b/src/AppInstallerCLITests/Runtime.cpp
@@ -8,7 +8,6 @@ using namespace AppInstaller;
using namespace AppInstaller::Runtime;
using namespace TestCommon;
-
bool CanWriteToPath(const std::filesystem::path& directory, const std::filesystem::path& file = "test.txt")
{
std::ofstream out{ directory / file };
@@ -94,3 +93,24 @@ TEST_CASE("ApplyACL_CurrentUserOwner_SystemAll", "[runtime]")
REQUIRE(CanWriteToPath(directory));
}
+
+TEST_CASE("VerifyDevModeEnabledCheck", "[runtime]")
+{
+ if (!Runtime::IsRunningAsAdmin())
+ {
+ WARN("Test requires admin privilege. Skipped.");
+ return;
+ }
+
+ bool initialState = IsDevModeEnabled();
+
+ EnableDevMode(!initialState);
+ bool modifiedState = IsDevModeEnabled();
+
+ // Revert to original state.
+ EnableDevMode(initialState);
+ bool revertedState = IsDevModeEnabled();
+
+ REQUIRE(modifiedState != initialState);
+ REQUIRE(revertedState == initialState);
+}+
\ No newline at end of file
diff --git a/src/AppInstallerCLITests/TestCommon.cpp b/src/AppInstallerCLITests/TestCommon.cpp
@@ -220,6 +220,13 @@ namespace TestCommon
THROW_IF_WIN32_ERROR(RegSetValueExW(key, name.c_str(), 0, REG_DWORD, reinterpret_cast<const BYTE*>(&value), sizeof(DWORD)));
}
+ void EnableDevMode(bool enable)
+ {
+ wil::unique_hkey result;
+ THROW_IF_WIN32_ERROR(RegOpenKeyExW(HKEY_LOCAL_MACHINE, L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\AppModelUnlock", 0, KEY_ALL_ACCESS|KEY_WOW64_64KEY, &result));
+ SetRegistryValue(result.get(), L"AllowDevelopmentWithoutDevLicense", (enable ? 1 : 0));
+ }
+
TestUserSettings::TestUserSettings(bool keepFileSettings)
{
if (!keepFileSettings)
diff --git a/src/AppInstallerCLITests/TestCommon.h b/src/AppInstallerCLITests/TestCommon.h
@@ -120,6 +120,9 @@ namespace TestCommon
void SetRegistryValue(HKEY key, const std::wstring& name, const std::vector<BYTE>& value, DWORD type = REG_BINARY);
void SetRegistryValue(HKEY key, const std::wstring& name, DWORD value);
+ // Enable or disable developer mode.
+ void EnableDevMode(bool enable);
+
// Override UserSettings using this class.
// Automatically overrides the user settings for the lifetime of this object.
// DOES NOT SUPPORT NESTED USE
diff --git a/src/AppInstallerCLITests/WorkFlow.cpp b/src/AppInstallerCLITests/WorkFlow.cpp
@@ -1217,6 +1217,40 @@ TEST_CASE("PortableInstallFlow", "[InstallFlow][workflow]")
REQUIRE(std::filesystem::exists(portableInstallResultPath.GetPath()));
}
+
+TEST_CASE("PortableInstallFlow_DevModeDisabled", "[InstallFlow][workflow]")
+{
+ if (!AppInstaller::Runtime::IsRunningAsAdmin())
+ {
+ WARN("Test requires admin privilege. Skipped.");
+ return;
+ }
+
+ TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false);
+ TestCommon::TempFile portableInstallResultPath("TestPortableInstalled.txt");
+
+ std::ostringstream installOutput;
+ TestContext context{ installOutput, std::cin };
+ auto previousThreadGlobals = context.SetForCurrentThread();
+ TestCommon::EnableDevMode(false);
+
+ // Override admin check to report as false.
+ context.Override({ EnsureRunningAsAdmin, [](TestContext& testContext)
+ {
+ testContext.SetTerminationHR(APPINSTALLER_CLI_ERROR_COMMAND_REQUIRES_ADMIN);
+ } });
+
+ context.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Portable.yaml").GetPath().u8string());
+ InstallCommand install({});
+ install.Execute(context);
+ TestCommon::EnableDevMode(true);
+ INFO(installOutput.str());
+
+ // Verify proper message is printed for installing portable in non-developer mode and not running as admin.
+ REQUIRE_FALSE(std::filesystem::exists(portableInstallResultPath.GetPath()));
+ REQUIRE(installOutput.str().find("https://github.com/microsoft/winget-cli/issues/2368") != std::string::npos);
+}
+
TEST_CASE("ShellExecuteHandlerInstallerArgs", "[InstallFlow][workflow]")
{
{
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h
@@ -125,6 +125,9 @@ namespace AppInstaller::Runtime
// Determines whether the process is running with administrator privileges.
bool IsRunningAsAdmin();
+ // Determines whether developer mode is enabled.
+ bool IsDevModeEnabled();
+
// Returns true if this is a release build; false if not.
inline constexpr bool IsReleaseBuild();
diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp
@@ -7,6 +7,7 @@
#include "Public/AppInstallerStrings.h"
#include "Public/winget/Filesystem.h"
#include "Public/winget/UserSettings.h"
+#include "Public/winget/Registry.h"
#define WINGET_DEFAULT_LOG_DIRECTORY "DiagOutputDir"
@@ -29,6 +30,8 @@ namespace AppInstaller::Runtime
constexpr std::string_view s_PortablePackageRoot = "WinGet"sv;
constexpr std::string_view s_PortablePackagesDirectory = "Packages"sv;
constexpr std::string_view s_LinksDirectory = "Links"sv;
+ constexpr std::string_view s_DevModeSubkey = "SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\AppModelUnlock"sv;
+ constexpr std::string_view s_AllowDevelopmentWithoutDevLicense = "AllowDevelopmentWithoutDevLicense"sv;
#ifndef WINGET_DISABLE_FOR_FUZZING
constexpr std::string_view s_SecureSettings_Relative_Packaged = "pkg"sv;
#endif
@@ -731,6 +734,22 @@ namespace AppInstaller::Runtime
return wil::test_token_membership(nullptr, SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
}
+ // Determines whether developer mode is enabled.
+ // Does not account for the group policy value which takes precedence over this registry value.
+ bool IsDevModeEnabled()
+ {
+ const auto& devModeSubKey = Registry::Key::OpenIfExists(HKEY_LOCAL_MACHINE, s_DevModeSubkey, 0, KEY_READ|KEY_WOW64_64KEY);
+ const auto& devModeEnabled = devModeSubKey[s_AllowDevelopmentWithoutDevLicense];
+ if (devModeEnabled.has_value())
+ {
+ return devModeEnabled->GetValue<Registry::Value::Type::DWord>() == 1;
+ }
+ else
+ {
+ return false;
+ }
+ }
+
constexpr bool IsReleaseBuild()
{
#ifdef WINGET_ENABLE_RELEASE_BUILD
