Fall back to hard link when symlink creation lacks privilege#6270
Fall back to hard link when symlink creation lacks privilege#6270rich-purnell wants to merge 4 commits into
Conversation
|
@microsoft-github-policy-service agree |
Trenly
left a comment
There was a problem hiding this comment.
The uninstall flow may need to account for hard links with this change. Tests should be added to validate hardlink creation, path resolution, and removal.
What happens in the upgrade scenario where a hardlink exists but the user now has symlink permissions - does it stay a hardlink, or should it be replaced with a symlink?
2d97bd6 to
2f8b7f5
Compare
I see that in the current installation flow, before creating a symlink, |
|
Hard links only work when the link and target are on the same NTFS volume, and they cannot link directories at all. WinGet needs cross volume and directory level linking for portable packages, and it must track and remove links cleanly during upgrades. Symlinks support all of that, hard links don’t. |
I was about to post a similar comment. However, I expect that a winget user is a more-than-average expert user, so they just need the proper guidance. I think the following should work:
|
On that note, this PR was created because the average WinGet user is often not aware they should be running in developer mode or with elevation. This PR is more prone to generating issue reports, and doesn't solve the root problem. Honestly, we should be solving the symlink problem directly with more robust error checking and handling. For instance, we can trigger an elevated command for the symlink (which will prompt UAC), if the initial link creation fails due to permissions issues. |
|
ok, sorry, I thought that UAC was not a possibility for some reason. Then I understand it is preferrable to my suggestion about elevated '--scope' command.
|
Keep in mind that the elevated command would only create the symlink, the package would still be installed in the user scope. WinGet already informs the user that it lacks permissions in its current implementation, https://github.com/microsoft/winget-cli/blob/master/src/AppInstallerSharedLib/Filesystem.cpp#L466 I do not disagree with anything you've said. I just don't believe this PR is the solution we should be looking for, and I mean no offense to the original poster. |
|
I see. I am not familiar with winget development. I hope I am not creating useless noise, please tell me if this is the case. Regarding your last comment
I did not have such experience (see my comment on 6058): no user-visible feedback was provided in case the symlink fails. Summarizing I think that requesting elevation is a good fix, but I would add a message explaining why elevation is required. |
As you can see, it reports both success and failure. Prior to this installation, I had always assumed that the aliases were successfully created, because your "if alias creation fails, add the app's installation directory to PATH" strategy effectively deceived me into thinking that. It was only when I noticed that Godot's program names did not match the aliases that I began to investigate the issue. At that point, I discovered that "symlink creation" had never worked on my system.
|
One core tenet of WinGet is that it must be able to install applications unattended - if there were to be a UAC prompt, it should come right at the beginning while a user is still at the machine; However, with installer selection being potentially deferred, especially in the case of |
|
What are all the limitations with hard links? Just being on the same NTFS volume? So: I think the last case would be extremely rare. The default locations for the executables and the links are both in |
|
Symlinks have an additional property; the actual executable is located in its directory that may contain dependencies like DLLs. So to the running code, it appears to be at its resolved location, not the symlink. A hardlink would appear to be running at the hardlink location (it is just another "normal" file that happens to have the same data stream as another file). One potential option for the issue of creating aliases would be to still add the location to PATH while creating the hardlinks in the target location (rather than the Links directory). |
This abstraction is not perfect, there are still cases where being a symlink is not the same as the real .exe. This issue just further proves that using small forwarder/shim exe files is the best solution to solve the %path% question (IMHO). |
Those are problematic from a signing perspective, not to mention all of the ways in which perfectly forwarding everything from one process to another is difficult/fragile. We should investigate the breadth of support for App Paths amongst users who are having issues with the current setup flow. If they are mostly using shells that integrate with this registration mechanism, it would probably be a better solution than continuing down the current path. Overall, it would probably be good to have many of these options (and CLI params), choose the best defaults, and allow for |
If you don't care about the icon, it would just be the same WinGet provided shim that is signed (and hardlinked for each new app).
The only thing you have to forward is the command line arguments, everything else (environment and current directory) is automatic. |
That's problematic at least for cases where you want to run as admin. If there is an UAC it should be "do you want to run Cool App, published by Contoso (verified by signature)?" If we create a shim that is specific to this app, we could not sign it, so the UAC would be "do you want to run Cool App, published by Contoso (unverified)?" Not having the publisher verified should be a red flag for people running this. If we use a common shim that we sign beforehand, the UAC would always say "do you want to run WinGet Shim, published by Microsoft (verified by signature)?" This could mislead people into thinking the app is trustworthy and from MS, not just a random app from who-knows-who. |
The shim would never request UAC elevation in its manifest. If someone enumerates %path% searching for a specific filename and then on purpose uses "runas" with ShellExecute to elevate then yes, you would get a elevation request on the shim itself. But yes, this is a case that is problematic. |
Remove InstallDirectoryAddedToPath check during symlink creation in installation. Symlink targets may reside in different directories, and the previous directory-empty check would always fail due to leftover *.db files, preventing proper environment variable cleanup. Add optional checkIfEmpty parameter (default: true) to RemoveFromPathVariable to control whether the target directory must exist and be empty before removing it from PATH. During uninstallation, when InstallDirectoryAddedToPath is true, skip the emptiness check and remove the environment variable unconditionally.
Implement a graceful fallback strategy for creating and removing aliases to handle permission issues and cross-device link failures. When creating a symlink: Attempt to create a symlink first; exit on success. If it fails due to insufficient permissions, proceed to step 2; otherwise, throw an exception. Fall back to creating a hard link; exit on success, otherwise proceed to step 3. Enter InstallDirectoryAddedToPath mode and add symlinkTargetPath.parent_path() to the PATH environment variable. When removing a symlink: If the target is identified as a symlink, delete it; otherwise, proceed to step 2. If the file exists and is a regular file (treated as a hard link), delete it; otherwise, proceed to step 3. Enter InstallDirectoryAddedToPath mode and remove symlinkTargetPath.parent_path() from the PATH environment variable.
2f8b7f5 to
8bc8d16
Compare
📖 Description
When WinGet runs without administrator privileges or Developer Mode enabled,
std::filesystem::create_symlink()fails with the error:This happens because creating symbolic links requires the
SeCreateSymbolicLinkPrivilege, which is only granted to administrators or available when Developer Mode is turned on. While WinGet typically runs with those privileges, there are scenarios where they are not held — for example, when running as a standard user or in a restricted environment.I wrote a local test program that confirmed this behavior. Under those conditions,
std::filesystem::create_symlink(target, link)fails with the above error, butstd::filesystem::create_hard_link(target, link)succeeds. Since a hard link provides identical file resolution behavior for this use case (pointing to the same file on disk), it is a viable fallback.This change adds a fallback: when symlink creation fails specifically with
ERROR_PRIVILEGE_NOT_HELD, the code attempts a hard link instead before returningfalse.Such a change may not solve all problems (for example, in some special file system structures), but it can reduce the impact scope.
🔗 References
Resolves #6058
🔍 Validation
Build succeeds with no errors or warnings.
✅ Checklist
📋 Issue Type
Microsoft Reviewers: Open in CodeFlow