Skip to content

Fall back to hard link when symlink creation lacks privilege #6270

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

Open
rich-purnell wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fall back to hard link when symlink creation lacks privilege #6270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0add6dc
Remove invalid CreateSymlink overload declaration
rich-purnell Jun 6, 2026
b8d7dfa
Remove invalid if branches
rich-purnell Jun 8, 2026
f367710
Improve symlink creation and PATH variable removal logic
rich-purnell Jun 12, 2026
b6930d7
Resolve alias creation failure with symlink/hardlink fallback
rich-purnell Jun 12, 2026
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
89 changes: 48 additions & 41 deletions src/AppInstallerCLICore/PortableInstaller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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))
Expand All @@ -157,8 +157,14 @@ namespace AppInstaller::CLI::Portable

if (Filesystem::CreateSymlink(symlinkTargetPath, filePath))
{
m_hasCreatedSymlink = true;
AICLI_LOG(Core, Info, << "Symlink created at: " << filePath << " with target path: " << symlinkTargetPath);
}
else if (Filesystem::CreateFileHardLink(symlinkTargetPath, filePath))
{
m_hasCreatedSymlink = true;
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.
Expand Down Expand Up @@ -187,18 +193,20 @@ namespace AppInstaller::CLI::Portable
{
AICLI_LOG(CLI, Info, << "Deleting portable symlink at: " << filePath);
std::filesystem::remove(filePath);
m_hasRemovedSymlink = true;
}
else if (InstallDirectoryAddedToPath)
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);
m_hasRemovedSymlink = true;
}
else
{
// 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);
Expand Down Expand Up @@ -294,7 +302,7 @@ namespace AppInstaller::CLI::Portable

ApplyDesiredState();

if (!InstallDirectoryAddedToPath)
if (m_hasCreatedSymlink)
{
AddToPathVariable(GetPortableLinksLocation(GetScope()));
}
Expand All @@ -307,20 +315,20 @@ namespace AppInstaller::CLI::Portable
}
}

void PortableInstaller::Uninstall()
{
ApplyDesiredState();
void PortableInstaller::Uninstall()
{
ApplyDesiredState();

RemoveInstallDirectory();
RemoveInstallDirectory();

if (!InstallDirectoryAddedToPath)
{
RemoveFromPathVariable(GetPortableLinksLocation(GetScope()));
}
if (m_hasRemovedSymlink)
{
RemoveFromPathVariable(GetPortableLinksLocation(GetScope()));
}

m_portableARPEntry.Delete();
AICLI_LOG(CLI, Info, << "PortableARPEntry deleted.");
}
m_portableARPEntry.Delete();
AICLI_LOG(CLI, Info, << "PortableARPEntry deleted.");
}

void PortableInstaller::CreateTargetInstallDirectory()
{
Expand Down Expand Up @@ -375,27 +383,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)
{
Expand Down
6 changes: 5 additions & 1 deletion src/AppInstallerCLICore/PortableInstaller.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace AppInstaller::CLI::Portable
bool InstallDirectoryCreated = false;
bool BinariesDependOnPath = false;
// If we fail to create a symlink, add install directory to PATH variable
// TODO: Variable is redundant, remove it.
bool InstallDirectoryAddedToPath = false;

bool IsUpdate = false;
Expand Down Expand Up @@ -94,6 +95,9 @@ namespace AppInstaller::CLI::Portable
AppInstaller::Manifest::AppsAndFeaturesEntry GetAppsAndFeaturesEntry();

private:
// True if any symlink creation succeeds. No persistence required.
bool m_hasCreatedSymlink = false;
bool m_hasRemovedSymlink = false;
PortableARPEntry m_portableARPEntry;
std::vector<AppInstaller::Portable::PortableFileEntry> m_desiredEntries;
std::vector<AppInstaller::Portable::PortableFileEntry> m_expectedEntries;
Expand All @@ -114,6 +118,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);
};
}
58 changes: 50 additions & 8 deletions src/AppInstallerCLITests/InstallFlow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -743,25 +743,67 @@ 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));
}

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));
}

TEST_CASE("PortableInstallFlow_UserScope", "[InstallFlow][workflow]")
Expand Down
17 changes: 17 additions & 0 deletions src/AppInstallerCLITests/TestHooks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ namespace AppInstaller
namespace Filesystem
{
void TestHook_SetCreateSymlinkResult_Override(bool* status);
void TestHook_SetCreateHardLinkResult_Override(bool* status);
}

namespace Archive
Expand Down Expand Up @@ -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)
Expand Down
53 changes: 47 additions & 6 deletions src/AppInstallerCLITests/UpdateFlow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,49 @@ 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());
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 };
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));
}

TEST_CASE("UpdateFlow_Portable_HardLinkCreationFail", "[UpdateFlow][workflow]")
{
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();
Expand All @@ -198,13 +238,13 @@ TEST_CASE("UpdateFlow_Portable_SymlinkCreationFail", "[UpdateFlow][workflow]")
update.Execute(context);
INFO(updateOutput.str());
const auto& portableTargetPath = targetDirectory / "AppInstallerTestExeInstaller.exe";
REQUIRE(std::filesystem::exists(portableTargetPath));
REQUIRE(AppInstaller::Registry::Environment::PathVariable(AppInstaller::Manifest::ScopeEnum::User).Contains(targetDirectory));
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 };
auto previousThreadGlobals = uninstallContext.SetForCurrentThread();
uninstallContext.SetForCurrentThread();
OverrideForCompositeInstalledSource(uninstallContext, CreateTestSource({ TSR::TestInstaller_Portable }));
uninstallContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Portable.Query);

Expand All @@ -213,6 +253,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]")
Expand Down
Loading
Loading