Check symlink target before removal (#2242)

microsoft · Jun 17, 2022 · d551743 · d551743
1 parent 5bdc55e
commit d551743
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 6 deletions.
diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h
@@ -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);

diff --git a/src/AppInstallerCLICore/Workflows/PortableFlow.cpp b/src/AppInstallerCLICore/Workflows/PortableFlow.cpp
@@ -451,11 +451,29 @@ namespace AppInstaller::CLI::Workflow
             const auto& symlinkPath = uninstallEntry[PortableValueName::PortableSymlinkFullPath];
             if (symlinkPath.has_value())
             {
-                const std::filesystem::path& symlinkPathValue = symlinkPath.value().GetValue<Value::Type::UTF16String>();
-
-                if (!std::filesystem::remove(symlinkPathValue))
+                const std::filesystem::path& symlinkPathValue = symlinkPath->GetValue<Value::Type::UTF16String>();
+                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->GetValue<Value::Type::UTF16String>();
+                        if (symlinkTargetPath != targetPathValue)
+                        {
+                            AICLI_LOG(CLI, Warning, << "Portable symlink not deleted; Symlink points to a different target exe: " << symlinkTargetPath <<
+                                "; 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

diff --git a/src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj b/src/AppInstallerCLIE2ETests/AppInstallerCLIE2ETests.csproj
@@ -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>

diff --git a/src/AppInstallerCLIE2ETests/UninstallCommand.cs b/src/AppInstallerCLIE2ETests/UninstallCommand.cs
@@ -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";
@@ -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]

diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
@@ -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>
+  </data>
 </root>