Add support for creating symlinks as an unprivileged user

[ralish]

This requires Windows 10 Creators Update (v1703) or newer as the underlying API support isn't present in earlier releases of Windows. Further, it's currently restricted by the OS to systems which have Developer Mode enabled. If these requirements aren't met, symlink handling remains unchanged from previous releases of Windows.

The support is provided by a new flag to CreateSymbolicLinkW:
SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE

More details are on the official developer blog announcement:
https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/

[PhilipOakley]

In the code ``` + if (has_unprivileged_symlinks == 1) { + symlink_flags |= SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;```The `== 1` step shouldn't use a magic number. I think `SYMBOLIC_LINK_FLAG_DIRECTORY` may be the right symbol as it is the initial setting of `symlink_flags`.--Philip(sorry about the top posting, it was the easiest way of retaining context - other ways were too hard in my email client)----- Original Message ----- From: Samuel D. Leslie To: git-for-windows/git Cc: Subscribed Sent: Sunday, May 28, 2017 4:46 AM Subject: [git-for-windows/git] Add support for creating symlinks as an unprivileged user (#1184) This requires Windows 10 Creators Update (v1703) or newer as the underlying API support isn't present in earlier releases of Windows. Further, it's currently restricted by the OS to systems which have Developer Mode enabled. If these requirements aren't met, symlink handling remains unchanged from previous releases of Windows. The support is provided by a new flag to CreateSymbolicLinkW: SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE More details are on the official developer blog announcement: https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/

…

------------------------------------------------------------------------------ You can view, comment on, or merge this pull request online at: #1184 Commit Summary a.. Add support for creating symlinks as an unprivileged user File Changes a.. M compat/mingw.c (38) Patch Links: a.. https://github.com/git-for-windows/git/pull/1184.patch b.. https://github.com/git-for-windows/git/pull/1184.diff

[ralish]

Thanks for the feedback Philip. To clarify, the has_unprivileged_symlinks variable is set on startup in the setup_windows_environment() function (near the end of the diff) based on whether the version of Windows we're running on has the requisite API support. The SYMBOLIC_LINK_FLAG_DIRECTORY symbol I actually added in as the original code was using the associated magic number, which was only going to get more confusing with the addition of the SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE flag. The latter is set only if has_unprivileged_symlinks is true (i.e. 1) from the earlier OS version check. In that sense this code should have less magic numbers than the original thanks to the #define lines for the actual flags?

[PhilipOakley]

Yes, given the extra #define of the symbol (SYMBOLIC_LINK_FLAG_DIRECTORY), the if statements should be updated to compare the has_unprivileged_symlinks value with the symbol (rather than it's magic number). It felt like an obvious tweak when I looked at the https://patch-diff.githubusercontent.com/raw/git-for-windows/git/pull/1184.patch version, which add the line. In the glorious git way, if there are more of those comparisons splattered about the code, splitting the patch into two so that the first patch does the preparatory #defines (noting the use of the second symbol will be in patch two), and then the second patch would be the substantive bulk of your "support for creating symlinks as an unprivileged user"

…

----- Original Message ----- From: Samuel D. Leslie Thanks for the feedback Philip. To clarify, the has_unprivileged_symlinks variable is set on startup in the setup_windows_environment() function (near the end of the diff) based on whether the version of Windows we're running on has the requisite API support. The SYMBOLIC_LINK_FLAG_DIRECTORY symbol I actually added in as the original code was using the associated magic number, which was only going to get more confusing with the addition of the SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE flag. The latter is set only if has_unprivileged_symlinks is true (i.e. 1) from the earlier OS version check. In that sense this code should have less magic numbers than the original thanks to the #defines of the actual flags?

[abrasive]

has_unprivileged_symlinks is a boolean. Semantically distinct from the magic flag value which is added if the boolean indicates support. If anything, perhaps if (has_unprivileged_symlinks) { might be the change you are looking for.

[dscho]

The really curious thing to me is: there is support in Git for Windows (actually, Git itself) to test whether symlinks can be created in the .git/ directory, and if they can be created, the core.symlinks flag is set to true. This happens upon git init (and implicitly in git clone).

Granted, this will not retroactively turn a non-symlink repository into a symlink one. But we should not do that automagically, either.

But the real question is: why is the current code not good enough? I did test the code previously, if the current account can create symbolic links, core.symlinks is set correctly and symbolic links are used instead of plain files.

[ralish]

@dscho Thanks for the review! To answer your question as to what this brings over the existing code, in most configurations creating symlinks on Windows requires Administrator privileges, with an elevated process if UAC is enabled. This code makes it possible to create symlinks as a non-Administrator user on the latest release of Windows 10 by taking advantage of the improvements made to the symbolic link APIs. In this respect, this (finally!) enables working with symlinks on Windows 10 without also having to work as an Administrator (ala. sudo), just like in most other operating systems.

The gory technical details:
Creating symlinks on Windows requires the SeCreateSymbolicLink privilege which is only assigned to Administrators by default. While you can change the local security policy to grant this privilege to other users or groups, this won't be sufficient if User Account Control is enabled (the default), as it will always filter the SeCreateSymbolicLink privilege even if it's granted to non-Administrator users. The end result is that working with symlinks on Windows requires running as an Administrator with an elevated process (e.g. Git Bash, Command Prompt, etc...) unless you disable UAC, which may not be desirable for several reasons.

The new flag we pass in, SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE, indicates to Windows that the application is aware of and supports creating symlinks without Administrator privileges if the underlying OS is configured to permit it. For now, this means Windows 10 Creators Update with Developer Mode enabled. If those requirements aren't met, there's no change, and the flag won't have any effect.

To test: if you try and checkout a repository with symlinks on a Windows 10 Creators Update system with Developer Mode enabled while running as a regular user, without this patch symlinks won't be created.

[dscho]

Thank you for the PR, the explanation and the patience.

I actually knew about the privileges, but I somehow missed the required flag.

While testing whether the flag 0x02 is acceptable before the Creators Update, I essentially reimplemented your patch in a different way (the version test has little to do with the environment, for example, so I moved the test to the startup function).

I also ran into a pretty nasty bug where symlink() would report success, but the link was not created. Turns out that we have a really ancient bug where the return type of the CreateSymbolicLinkW() was incorrect, leading to possibly undefined behavior. Which I promptly encountered.

Could you have a look at #1184?

[ralish]

I think you mean #1188 :)

[dscho]

Oops. Yes. 🤦

[dscho]

Superseded by #1188.