Fall back to hard link when symlink creation lacks privilege

src/AppInstallerCLICore/PortableInstaller.cpp

@@ -126,7 +126,7 @@ namespace AppInstaller::CLI::Portable
         {
             std::filesystem::path symlinkTargetPath{ Utility::ConvertToUTF16(entry.SymlinkTarget) };
 
-            if (BinariesDependOnPath && !InstallDirectoryAddedToPath)
+            if (BinariesDependOnPath)
             {
                 // Scenario indicated by 'ArchiveBinariesDependOnPath' manifest entry.
                 // Skip symlink creation for portables dependent on binaries that require the install directory to be added to PATH.
@@ -135,7 +135,7 @@ namespace AppInstaller::CLI::Portable
                 AICLI_LOG(Core, Info, << "Install directory added to PATH: " << installDirectory);
                 CommitToARPEntry(PortableValueName::InstallDirectoryAddedToPath, InstallDirectoryAddedToPath = true);
             }
-            else if (!InstallDirectoryAddedToPath)
+            else
             {
                 std::filesystem::file_status status = std::filesystem::status(filePath);
                 if (std::filesystem::is_directory(status))
@@ -159,6 +159,10 @@ namespace AppInstaller::CLI::Portable
                 {
                     AICLI_LOG(Core, Info, << "Symlink created at: " << filePath << " with target path: " << symlinkTargetPath);
                 }
+                else if (Filesystem::CreateFileHardLink(symlinkTargetPath, filePath))
+                {
+					AICLI_LOG(Core, Info, << "Hardlink created at: " << filePath << " with target path: " << symlinkTargetPath);
+                }
                 else
                 {
                     // If symlink creation fails, resort to adding the package directory to PATH.
@@ -188,17 +192,17 @@ namespace AppInstaller::CLI::Portable
                 AICLI_LOG(CLI, Info, << "Deleting portable symlink at: " << filePath);
                 std::filesystem::remove(filePath);
             }
+            else if (std::filesystem::exists(filePath) && std::filesystem::is_regular_file(std::filesystem::status(filePath)))
+            {
+				AICLI_LOG(CLI, Info, << "Deleting portable hard link at: " << filePath);
+				std::filesystem::remove(filePath);
+            }
             else if (InstallDirectoryAddedToPath)
             {
                 // If symlink doesn't exist, check if install directory was added to PATH directly and remove.
-                RemoveFromPathVariable(std::filesystem::path(Utility::ConvertToUTF16(entry.SymlinkTarget)).parent_path());
+                RemoveFromPathVariable(std::filesystem::path(Utility::ConvertToUTF16(entry.SymlinkTarget)).parent_path(), false);
             }
         }
-        else if (fileType == PortableFileType::Symlink && Filesystem::SymlinkExists(filePath))
-        {
-            AICLI_LOG(CLI, Info, << "Deleting portable symlink at: " << filePath);
-            std::filesystem::remove(filePath);
-        }
         else if (fileType == PortableFileType::Directory && std::filesystem::exists(filePath))
         {
             AICLI_LOG(CLI, Info, << "Removing directory at " << filePath);
@@ -307,20 +311,20 @@ namespace AppInstaller::CLI::Portable
         }
     }
 
-    void PortableInstaller::Uninstall()
-    {
-        ApplyDesiredState();
+	void PortableInstaller::Uninstall()
+	{
+		ApplyDesiredState();
 
-        RemoveInstallDirectory();
+		RemoveInstallDirectory();
 
-        if (!InstallDirectoryAddedToPath)
-        {
-            RemoveFromPathVariable(GetPortableLinksLocation(GetScope()));
-        }
+		if (!InstallDirectoryAddedToPath)
+		{
+			RemoveFromPathVariable(GetPortableLinksLocation(GetScope()));
+		}
 
-        m_portableARPEntry.Delete();
-        AICLI_LOG(CLI, Info, << "PortableARPEntry deleted.");
-    }
+		m_portableARPEntry.Delete();
+		AICLI_LOG(CLI, Info, << "PortableARPEntry deleted.");
+	}
 
     void PortableInstaller::CreateTargetInstallDirectory()
     {
@@ -375,27 +379,26 @@ namespace AppInstaller::CLI::Portable
         }
     }
 
-    void PortableInstaller::RemoveFromPathVariable(std::filesystem::path value)
-    {
-        if (std::filesystem::exists(value) && !std::filesystem::is_empty(value))
-        {
-            AICLI_LOG(Core, Info, << "Install directory is not empty: " << value);
-        }
-        else
-        {
+	void PortableInstaller::RemoveFromPathVariable(std::filesystem::path value, bool checkIfEmpty /*= true*/)
+	{
+		if (checkIfEmpty && std::filesystem::exists(value) && !std::filesystem::is_empty(value))
+		{
+			AICLI_LOG(Core, Info, << "Install directory is not empty: " << value);
+		}
+		else
+		{
 			// Attempt to remove both the original and the preferred format to ensure removal
-            // Necessary for handling old path values associated with winget-cli#5033
-            if (PathVariable(GetScope()).Remove(value) || PathVariable(GetScope()).Remove(value.make_preferred()))
-            {
-                InstallDirectoryAddedToPath = false;
-                AICLI_LOG(CLI, Info, << "Removed target directory from PATH registry: " << value);
-            }
-            else
-            {
-                AICLI_LOG(CLI, Info, << "Target directory not removed from PATH registry: " << value);
-            }
-        }
-    }
+			// Necessary for handling old path values associated with winget-cli#5033
+			if (PathVariable(GetScope()).Remove(value) || PathVariable(GetScope()).Remove(value.make_preferred()))
+			{
+				AICLI_LOG(CLI, Info, << "Removed target directory from PATH registry: " << value);
+			}
+			else
+			{
+				AICLI_LOG(CLI, Info, << "Target directory not removed from PATH registry: " << value);
+			}
+		}
+	}
 
     void PortableInstaller::SetAppsAndFeaturesMetadata(const Manifest::Manifest& manifest, const std::vector<AppInstaller::Manifest::AppsAndFeaturesEntry>& entries)
     {

src/AppInstallerCLICore/PortableInstaller.h

@@ -114,6 +114,6 @@ namespace AppInstaller::CLI::Portable
         void RemoveInstallDirectory();
 
         void AddToPathVariable(std::filesystem::path value);
-        void RemoveFromPathVariable(std::filesystem::path value);
+        void RemoveFromPathVariable(std::filesystem::path value, bool checkIfEmpty = true);
     };
 }

src/AppInstallerCLITests/InstallFlow.cpp

@@ -731,7 +731,7 @@ TEST_CASE("InstallFlow_Portable_SymlinkCreationFail", "[InstallFlow][workflow]")
     TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false);
     std::ostringstream installOutput;
     TestContext installContext{ installOutput, std::cin };
-    auto PreviousThreadGlobals = installContext.SetForCurrentThread();
+    installContext.SetForCurrentThread();
     OverridePortableInstaller(installContext);
     TestHook::SetCreateSymlinkResult_Override createSymlinkResultOverride(false);
     const auto& targetDirectory = tempDirectory.GetPath();
@@ -743,25 +743,69 @@ TEST_CASE("InstallFlow_Portable_SymlinkCreationFail", "[InstallFlow][workflow]")
     InstallCommand install({});
     install.Execute(installContext);
 
-    {
-        INFO(installOutput.str());
+	AppInstaller::CLI::Portable::PortableInstaller& portableInstaller = installContext.Get<Execution::Data::PortableInstaller>();
+	std::filesystem::path portableLinksLocation = AppInstaller::CLI::Portable::GetPortableLinksLocation(portableInstaller.GetScope());
+    std::filesystem::path symlinkFile = portableLinksLocation / "AppInstallerTestExeInstaller.exe";
 
-        // Use CHECK to allow the uninstall to still occur
-        CHECK(std::filesystem::exists(portableTargetPath));
-        CHECK(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(targetDirectory));
-    }
+	INFO(installOutput.str());
+
+	// Use CHECK to allow the uninstall to still occur
+	CHECK(std::filesystem::exists(portableTargetPath));
+
+	CHECK(std::filesystem::exists(symlinkFile));
+	CHECK(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(portableLinksLocation));
 
     // Perform uninstall
     std::ostringstream uninstallOutput;
     TestContext uninstallContext{ uninstallOutput, std::cin };
-    auto previousThreadGlobals = uninstallContext.SetForCurrentThread();
+    uninstallContext.SetForCurrentThread();
     uninstallContext.Args.AddArg(Execution::Args::Type::Name, "AppInstaller Test Portable Exe"sv);
     uninstallContext.Args.AddArg(Execution::Args::Type::AcceptSourceAgreements);
 
     UninstallCommand uninstall({});
     uninstall.Execute(uninstallContext);
     INFO(uninstallOutput.str());
     REQUIRE_FALSE(std::filesystem::exists(portableTargetPath));
+    REQUIRE_FALSE(std::filesystem::exists(symlinkFile));
+    REQUIRE_FALSE(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(portableLinksLocation));
+}
+
+TEST_CASE("InstallFlow_Portable_HardLinkCreationFail", "[InstallFlow][workflow]")
+{
+	TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false);
+	std::ostringstream installOutput;
+	TestContext installContext{ installOutput, std::cin };
+	installContext.SetForCurrentThread();
+	OverridePortableInstaller(installContext);
+	TestHook::SetCreateSymlinkResult_Override createSymlinkResultOverride(false);
+	TestHook::SetCreateHardLinkResult_Override createHardLinkResultOverride(false);
+	const auto& targetDirectory = tempDirectory.GetPath();
+	const auto& portableTargetPath = targetDirectory / "AppInstallerTestExeInstaller.exe";
+	installContext.Args.AddArg(Execution::Args::Type::Manifest, TestDataFile("InstallFlowTest_Portable.yaml").GetPath().u8string());
+	installContext.Args.AddArg(Execution::Args::Type::InstallLocation, targetDirectory.u8string());
+	installContext.Args.AddArg(Execution::Args::Type::InstallScope, "user"sv);
+
+	InstallCommand install({});
+	install.Execute(installContext);
+
+	INFO(installOutput.str());
+
+	// Use CHECK to allow the uninstall to still occur
+	CHECK(std::filesystem::exists(portableTargetPath));
+	CHECK(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(targetDirectory));
+
+	// Perform uninstall
+	std::ostringstream uninstallOutput;
+	TestContext uninstallContext{ uninstallOutput, std::cin };
+	uninstallContext.SetForCurrentThread();
+	uninstallContext.Args.AddArg(Execution::Args::Type::Name, "AppInstaller Test Portable Exe"sv);
+	uninstallContext.Args.AddArg(Execution::Args::Type::AcceptSourceAgreements);
+
+	UninstallCommand uninstall({});
+	uninstall.Execute(uninstallContext);
+	INFO(uninstallOutput.str());
+	REQUIRE_FALSE(std::filesystem::exists(portableTargetPath));
+    REQUIRE_FALSE(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(targetDirectory));
 }
 
 TEST_CASE("PortableInstallFlow_UserScope", "[InstallFlow][workflow]")

src/AppInstallerCLITests/TestHooks.h

@@ -68,6 +68,7 @@ namespace AppInstaller
     namespace Filesystem
     {
         void TestHook_SetCreateSymlinkResult_Override(bool* status);
+        void TestHook_SetCreateHardLinkResult_Override(bool* status);
     }
 
     namespace Archive
@@ -138,6 +139,22 @@ namespace TestHook
         bool m_status;
     };
 
+	struct SetCreateHardLinkResult_Override
+	{
+		SetCreateHardLinkResult_Override(bool status) : m_status(status)
+		{
+			AppInstaller::Filesystem::TestHook_SetCreateHardLinkResult_Override(&m_status);
+		}
+
+		~SetCreateHardLinkResult_Override()
+		{
+			AppInstaller::Filesystem::TestHook_SetCreateHardLinkResult_Override(nullptr);
+		}
+
+	private:
+		bool m_status;
+	};
+
     struct SetScanArchiveResult_Override
     {
         SetScanArchiveResult_Override(bool status) : m_status(status)

src/AppInstallerCLITests/UpdateFlow.cpp

@@ -184,9 +184,51 @@ TEST_CASE("UpdateFlow_Portable_SymlinkCreationFail", "[UpdateFlow][workflow]")
     TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false);
     std::ostringstream updateOutput;
     TestContext context{ updateOutput, std::cin };
-    auto PreviousThreadGlobals = context.SetForCurrentThread();
-    bool overrideCreateSymlinkStatus = false;
-    AppInstaller::Filesystem::TestHook_SetCreateSymlinkResult_Override(&overrideCreateSymlinkStatus);
+    context.SetForCurrentThread();
+    TestHook::SetCreateSymlinkResult_Override createSymlinkResultOverride(false);
+    OverridePortableInstaller(context);
+    OverrideForCompositeInstalledSource(context, CreateTestSource({ TSR::TestInstaller_Portable }));
+    const auto& targetDirectory = tempDirectory.GetPath();
+    context.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Portable.Query);
+    context.Args.AddArg(Execution::Args::Type::InstallLocation, targetDirectory.u8string());
+    context.Args.AddArg(Execution::Args::Type::InstallScope, "user"sv);
+
+    UpgradeCommand update({});
+    update.Execute(context);
+
+    // Get the PortableInstaller to access the scope information
+    AppInstaller::CLI::Portable::PortableInstaller& portableInstaller = context.Get<Execution::Data::PortableInstaller>();
+    std::filesystem::path portableLinksLocation = AppInstaller::CLI::Portable::GetPortableLinksLocation(portableInstaller.GetScope());
+    std::filesystem::path symlinkFile = portableLinksLocation / "AppInstallerTestExeInstaller.exe";
+
+    INFO(updateOutput.str());
+    REQUIRE(std::filesystem::exists(symlinkFile));
+    REQUIRE(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(portableLinksLocation));
+
+    // Perform uninstall
+    std::ostringstream uninstallOutput;
+    TestContext uninstallContext{ uninstallOutput, std::cin };
+    uninstallContext.SetForCurrentThread();
+    OverrideForCompositeInstalledSource(uninstallContext, CreateTestSource({ TSR::TestInstaller_Portable }));
+    uninstallContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Portable.Query);
+
+    UninstallCommand uninstall({});
+    uninstall.Execute(uninstallContext);
+    INFO(uninstallOutput.str());
+
+    REQUIRE_FALSE(std::filesystem::exists(symlinkFile));
+    REQUIRE_FALSE(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(portableLinksLocation));
+}
+
+TEST_CASE("UpdateFlow_Portable_HardLinkCreationFail", "[UpdateFlow][workflow]")
+{
+    // Update portable with symlink creation failure verify that it succeeds.
+    TestCommon::TempDirectory tempDirectory("TestPortableInstallRoot", false);
+    std::ostringstream updateOutput;
+    TestContext context{ updateOutput, std::cin };
+    context.SetForCurrentThread();
+    TestHook::SetCreateSymlinkResult_Override createSymlinkResultOverride(false);
+    TestHook::SetCreateHardLinkResult_Override createHardLinkResultOverride(false);
     OverridePortableInstaller(context);
     OverrideForCompositeInstalledSource(context, CreateTestSource({ TSR::TestInstaller_Portable }));
     const auto& targetDirectory = tempDirectory.GetPath();
@@ -204,7 +246,7 @@ TEST_CASE("UpdateFlow_Portable_SymlinkCreationFail", "[UpdateFlow][workflow]")
     // Perform uninstall
     std::ostringstream uninstallOutput;
     TestContext uninstallContext{ uninstallOutput, std::cin };
-    auto previousThreadGlobals = uninstallContext.SetForCurrentThread();
+    uninstallContext.SetForCurrentThread();
     OverrideForCompositeInstalledSource(uninstallContext, CreateTestSource({ TSR::TestInstaller_Portable }));
     uninstallContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Portable.Query);
 
@@ -213,6 +255,7 @@ TEST_CASE("UpdateFlow_Portable_SymlinkCreationFail", "[UpdateFlow][workflow]")
     INFO(uninstallOutput.str());
 
     REQUIRE_FALSE(std::filesystem::exists(portableTargetPath));
+    REQUIRE_FALSE(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(targetDirectory));
 }
 
 TEST_CASE("UpdateFlow_UpdateExeWithUnsupportedArgs", "[UpdateFlow][workflow]")

src/AppInstallerSharedLib/Filesystem.cpp

@@ -474,6 +474,34 @@ namespace AppInstaller::Filesystem
         }
     }
 
+#ifndef AICLI_DISABLE_TEST_HOOKS
+    static bool* s_CreateHardLinkResult_TestHook_Override = nullptr;
+
+    void TestHook_SetCreateHardLinkResult_Override(bool* status)
+    {
+        s_CreateHardLinkResult_TestHook_Override = status;
+    }
+#endif
+
+	bool CreateFileHardLink(const std::filesystem::path& target, const std::filesystem::path& link)
+	{
+#ifndef AICLI_DISABLE_TEST_HOOKS
+        if (s_CreateHardLinkResult_TestHook_Override)
+        {
+            return *s_CreateHardLinkResult_TestHook_Override;
+        }
+#endif
+		try
+		{
+			std::filesystem::create_hard_link(target, link);
+			return true;
+		}
+		catch (...)
+		{
+			return false;
+		}
+	}
+
     bool VerifySymlink(const std::filesystem::path& symlink, const std::filesystem::path& target)
     {
         // Use read_symlink to get the symlink's recorded target without traversing the filesystem

src/AppInstallerSharedLib/Public/winget/Filesystem.h

@@ -30,6 +30,9 @@ namespace AppInstaller::Filesystem
     // Creates a symlink that points to the target path.
     bool CreateSymlink(const std::filesystem::path& target, const std::filesystem::path& link);
 
+	// Creates a hard link that points to the target path.
+	bool CreateFileHardLink(const std::filesystem::path& target, const std::filesystem::path& link);
+
     // Verifies that a symlink points to the target path.
     bool VerifySymlink(const std::filesystem::path& symlink, const std::filesystem::path& target);
 
@@ -38,7 +41,6 @@ namespace AppInstaller::Filesystem
 
     // Checks if the path is a symlink and exists.
     bool SymlinkExists(const std::filesystem::path& symlinkPath);
-    bool CreateSymlink(const std::filesystem::path& path, const std::filesystem::path& target);
 
     // Get expanded file system path.
     std::filesystem::path GetExpandedPath(const std::string& path);