Skip to content
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.

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

Check symlink target before removal #2242

Merged
merged 4 commits into from
Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ namespace AppInstaller::CLI::Resource
WINGET_DEFINE_RESOURCE_STRINGID(SourceUpdateCommandShortDescription);
WINGET_DEFINE_RESOURCE_STRINGID(SourceUpdateOne);
WINGET_DEFINE_RESOURCE_STRINGID(SystemArchitecture);
WINGET_DEFINE_RESOURCE_STRINGID(SymlinkModified);
WINGET_DEFINE_RESOURCE_STRINGID(TagArgumentDescription);
WINGET_DEFINE_RESOURCE_STRINGID(ThankYou);
WINGET_DEFINE_RESOURCE_STRINGID(ThirdPartSoftwareNotices);
Expand Down
24 changes: 21 additions & 3 deletions src/AppInstallerCLICore/Workflows/PortableFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,28 @@ namespace AppInstaller::CLI::Workflow
if (symlinkPath.has_value())
{
const std::filesystem::path& symlinkPathValue = symlinkPath.value().GetValue<Value::Type::UTF16String>();

if (!std::filesystem::remove(symlinkPathValue))
if (std::filesystem::is_symlink(std::filesystem::symlink_status(symlinkPathValue)))
{
AICLI_LOG(CLI, Info, << "Portable symlink not found; Unable to delete portable symlink: " << symlinkPathValue);
const auto& targetPath = uninstallEntry[PortableValueName::PortableTargetFullPath];
if (targetPath.has_value())
{
const std::filesystem::path& symlinkTargetPath = std::filesystem::read_symlink(symlinkPathValue);
const std::filesystem::path& targetPathValue = targetPath.value().GetValue<Value::Type::UTF16String>();
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
if (symlinkTargetPath != targetPathValue)
{
AICLI_LOG(CLI, Info, << "Portable symlink not deleted; Symlink points to a different target exe: " << symlinkTargetPath <<
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
"; Expected target exe: " << targetPathValue);
context.Reporter.Warn() << Resource::String::SymlinkModified << std::endl;
}
else if (!std::filesystem::remove(symlinkPathValue))
{
AICLI_LOG(CLI, Info, << "Portable symlink not found; Unable to delete portable symlink: " << symlinkPathValue);
}
}
else
{
AICLI_LOG(CLI, Info, << "The registry value for [TargetFullPath] does not exist");
}
}
}
else
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net5.0-windows</TargetFramework>
<TargetFramework>net6.0-windows</TargetFramework>
<OutDir>$(SolutionDir)$(Platform)\$(Configuration)\AppInstallerCLIE2ETests\</OutDir>
<IsPackable>false</IsPackable>
<Platforms>x64;x86</Platforms>
Expand Down
29 changes: 28 additions & 1 deletion src/AppInstallerCLIE2ETests/UninstallCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public void UninstallPortable()
[Test]
public void UninstallPortableWithProductCode()
{
// Uninstall a Portable
// Uninstall a Portable with ProductCode
string installDir = Path.Combine(System.Environment.GetEnvironmentVariable("LocalAppData"), "Microsoft", "WinGet", "Packages");
string packageId, commandAlias, fileName, packageDirName, productCode;
packageId = "AppInstallerTest.TestPortableExe";
Expand All @@ -92,6 +92,33 @@ public void UninstallPortableWithProductCode()
Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
Assert.True(result.StdOut.Contains("Successfully uninstalled"));
TestCommon.VerifyPortablePackage(Path.Combine(installDir, packageDirName), commandAlias, fileName, productCode, false);
}

[Test]
public void UninstallPortableModifiedSymlink()
{
string packageId, commandAlias;
packageId = "AppInstallerTest.TestPortableExe";
commandAlias = "AppInstallerTestExeInstaller.exe";

TestCommon.RunAICLICommand("install", $"{packageId}");

string symlinkDirectory = Path.Combine(System.Environment.GetEnvironmentVariable("LocalAppData"), "Microsoft", "WinGet", "Links");
string symlinkPath = Path.Combine(symlinkDirectory, commandAlias);

// Replace symlink with modified symlink
File.Delete(symlinkPath);
FileSystemInfo modifiedSymlinkInfo = File.CreateSymbolicLink(symlinkPath, "fakeTargetExe");
var result = TestCommon.RunAICLICommand("uninstall", $"{packageId}");

// Remove modified symlink as to not interfere with other tests
bool modifiedSymlinkExists = modifiedSymlinkInfo.Exists;
modifiedSymlinkInfo.Delete();

Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
Assert.True(result.StdOut.Contains("Successfully uninstalled"));
Assert.True(result.StdOut.Contains("Portable symlink not deleted as it was modified and points to a different target exe"));
Assert.True(modifiedSymlinkExists, "Modified symlink should still exist");
}

[Test]
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Original file line number Diff line number Diff line change
Expand Up @@ -1349,4 +1349,7 @@ Please specify one of them using the `--source` option to proceed.</value>
<data name="ShowLabelInstallationNotes" xml:space="preserve">
<value>InstallationNotes:</value>
</data>
<data name="SymlinkModified" xml:space="preserve">
<value>Portable symlink not deleted as it was modified and points to a different target exe</value>
ryfu-msft marked this conversation as resolved.
Show resolved Hide resolved
</data>
</root>