[Dev] Portable In Zip does not uninstall correctly

[Trenly]

Brief description of your issue

When a portable installer is installed from inside a Zip installer, if a user has specified the relative path to the file with a ./ preceding it, as is commonplace when representing paths relative to a given root, the file cannot be uninstalled correctly

Details on RCA

In `./src/AppInstallerCLICore/PortableInstaller.cpp` replace the `VerifyPortableFile` function on line 56 with this: ``` bool VerifyPortableFile(AppInstaller::Portable::PortableFileEntry& entry) { std::filesystem::path filePath = entry.GetFilePath(); PortableFileType fileType = entry.FileType; if (fileType == PortableFileType::File) { SHA256::HashBuffer fileHash = SHA256::ComputeHashFromFile(filePath); if (std::filesystem::exists(filePath) && !SHA256::AreEqual(fileHash, SHA256::ConvertToBytes(entry.SHA256))) { AICLI_LOG(CLI, Info, << "File hash does not match ARP Entry. Expected: " << entry.SHA256 << " Actual: " << fileHash.data()); return false; } } else if (fileType == PortableFileType::Symlink) { if (Filesystem::SymlinkExists(filePath) && !Filesystem::VerifySymlink(filePath, entry.SymlinkTarget)) { AICLI_LOG(CLI, Info, << "Symlink target does not match ARP Entry. Expected: " << entry.SymlinkTarget << " Actual: " << std::filesystem::read_symlink(filePath).string()); return false; } } return true; } ``` With the information the additional logging provided, I found this - > Symlink target does not match ARP Entry. Expected: C:\Users\Trenly\AppData\Local\Microsoft\WinGet\Packages\WindowsPostInstallWizard.UniversalSilentSwitchFinder_Microsoft.Winget.Source_8wekyb3d8bbwe\ussf.exe Actual: C:\Users\Trenly\AppData\Local\Microsoft\WinGet\Packages\WindowsPostInstallWizard.UniversalSilentSwitchFinder_Microsoft.Winget.Source_8wekyb3d8bbwe\\.\ussf.exe

Steps to reproduce

• Run wingetdev install ussf
• Wait for package to install
• Run wingetdev uninstall ussf

Expected behavior

Package uninstalls correctly

Actual behavior

Unable to remove Portable package as it has been modified; to override this check use --force

Environment

Wingetdev built off of https://github.com/microsoft/winget-cli/commit/8a8ad84c47705971b62492808800045c6f91961f

[Trenly]

@denelon - This is caused by the RelativeFilePath in the manifest being ./ussf.exe instead of ussf.exe. I'm not sure how you want to handle this - if it's an extra layer of validation that needs to be added, or if someone can make a quick change to parse the paths before comparing them.

In terms of execution the ./ doesn't seem to matter much since calling ussf from the command line after installing it works just fine.

I'd like to voice my opinion that I'm most comfortable using ./ notation as it frames in my mind the path from the current directory, so if possible I would like to see this solved by not restricting the RelativeFilePath

[denelon]

We had some concerns regarding the use of ../ to try and manipulate the path so it might be related to that. I'll let @ryfu-msft chime in.

[Trenly]

We had some concerns regarding the use of ../ to try and manipulate the path so it might be related to that. I'll let @ryfu-msft chime in.

I don’t think its related to that, since it isn’t triggering this check, which happens before the file is installed and the symlink is created

https://github.com/microsoft/winget-cli/blob/e0ac9653f30b93847784d43b56210d8e4a47eec4/src/AppInstallerCLICore/Workflows/ArchiveFlow.cpp#L57-L62

[ryfu-msft]

I still need to investigate this behavior further, but we currently don't do any processing on the portable command alias and simply append it to the symlink path. I wouldn't be surprised the additional .\ is being appended to the path and therefore not getting cleaned up correctly. I'll create a separate PR to address this issue as I do agree that we should put our best effort to make the path consistent.

[Trenly]

Symlink target does not match ARP Entry. Expected: C:\Users\Trenly\AppData\Local\Microsoft\WinGet\Packages\WindowsPostInstallWizard.UniversalSilentSwitchFinder_Microsoft.Winget.Source_8wekyb3d8bbwe\ussf.exe Actual: C:\Users\Trenly\AppData\Local\Microsoft\WinGet\Packages\WindowsPostInstallWizard.UniversalSilentSwitchFinder_Microsoft.Winget.Source_8wekyb3d8bbwe.\ussf.exe

I think that's what using the weakly_canonical will fix, since the symlink target was set to C:\Users\Trenly\AppData\Local\Microsoft\WinGet\Packages\WindowsPostInstallWizard.UniversalSilentSwitchFinder_Microsoft.Winget.Source_8wekyb3d8bbwe\.\ussf.exe. By using the weakly_canonical in the symlink creation, it should resolve the target before it creates the symlink

[ryfu-msft]

One thing to point out is that weakly_canonical resolves symlinks if the symlink exists. This means we'd have to ensure that the symlink does not already exist in order to reliably use weakly_canonical or else we could risk obtaining/utilizing the target exe path instead.