This proposal is interesting. I haven't realized till now that GetFinalPathNameByHandle is the root of most devil in filepath.EvalSymlinks.

I've done a quick prototype of your proposal (see CL 537295) and added a bunch of tests to make sure they fix #39786 and #40176. Everything seems to work fine with just one modification to the proposal: os.Readlink evaluate the target of a symbolic link AND replaces any NT prefix (\??\) with its Win32 counterpart. This means the following:

• Volumes with a device letter assigned get the NT prefix stripped. E.g. \??\C:\foo\bar => C:\foo\bar
• UNC NT path prefix is replaces with the UNC path prefix. E.g. \??\UNC\foo\bar => \\foo\bar
• Volumes without a drive letter (i.e. mount points) get the NT prefix replaced with the Win32 extended path prefix. E.g. \??\Volume{abc}\foo\bar => \\?\Volume{abc}\foo\bar.

Notice the first two bullets are already implemented by os.Readlink. This is necessary so the resulting path can interoperate with other Win32 APIs, which normally don't support the NT path prefix.

Bonus point: the new behavior seems to be an alternative fix for the bug that was originally fixed by making os.Readlink use GetFinalPathNameByHandle in CL 164201.

[edit: I no longer think this; see later comments]

I think I would still rather have Readlink itself only read links, which I believe would also keep #30463 fixed (because as far as I can tell it doesn't involve any actual symlinks). On Unix platforms, readlink only reads one level of link — it doesn't do anything with earlier elements in the path.

But, given that I'm proposing for EvalSymlinks to keep its case-conversion behavior, I think it would be reasonable for EvalSymlinks to also convert the NT prefix to its Win32 counterpart. That would still make EvalSymlinks do more than just evaluate symlinks, but at least it confines the extra transformations to within one function.

Especially given #57766, I suspect that the vast majority of existing callers are using filepath.EvalSymlinks rather than os.Readlink directly, so moving the transformation into EvalSymlinks probably won't affect many (if any) real programs.

From reading #30463, I believe that it was filed in response to containerd/continuity#113, which I believe is better addressed by https://go.dev/cl/463177.

(The current code in github.com/containerd/continuity appears to incorrectly assume that os.Readlink returns an absolute or relative version of the requested path, which is not what it actually does on Unix; see #57766. For that, I have filed containerd/continuity#232.)

Given that continuity.context.Walk is already (still) buggy, I don't think we should worry too much about maintaining the behavior established for it in #30463.

I think this proposal would address the underlying problem described in #29746 (comment) in a different way: namely, by not trying to resolve the C:\Data mount point as a symlink in the first place.

I think I would still rather have Readlink itself only read links, which I believe would also keep #30463 fixed (because as far as I can tell it doesn't involve any actual symlinks). On Unix platforms, readlink only reads one level of link — it doesn't do anything with earlier elements in the path.

Good enough 👍.

About changing os.Readlink to just read the link, what if we partially revert CL 164201 and implement the fix in syscall.Readlink instead (gated by godebug, if necessary). This way we will have just one definition of what reading a link does, and users calling syscall.Readlink will also benefit from having consitiency across OSes.

Actually, on further reading I think you are correct that os.Readlink itself should return a win32 path: its argument is generally a win32 path, so its return value should be as well. (The exact format of the reparse data buffer should be an implementation detail.)

I think we should deprecate syscall.Readlink entirely on Windows. (As far as I can tell, the Win32 API does not actually provide a readlink system call at all. The syscall package is supposed to provide low-level system calls, not platform-independent wrappers — those belong in os, which is what I am proposing to fix.)

Concretely: if a path with an NT prefix is passed to os.Readlink as its argument, it shouldn't make any particular effort to strip that prefix.

But if the argument refers to a symlink, and the symlink's substitute path includes an NT prefix, then os.Readlink should return it as a Win32 path instead.

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

Can someone briefly summarize the suggested changes here?

I may have misunderstood some comments above, but while I'm writing: We should not deprecate syscall.Readlink just because there is no Windows readlink system call. Strictly speaking there are no Windows system calls at all (that we are allowed to use), and the DLL calls that do exist definitely do not line up with the syscall API. Instead package syscall shoehorns Windows as best it can into a Unix-shaped boot. Even Open and Close are not Windows system calls or DLL calls. The fact that syscall.Readlink already exists and mostly matches os.Readlink suggests that it should remain that way, although it would be very good to deduplicate all that code into syscall (perhaps leaving fixLongPath in the os side).

The changes are:

• os.Lstat and os.Readlink stop treating mount points as symlinks, and instead report them as irregular files.
• os.Stat continues to resolve the underlying directory for a mount point (as it does today).
• os.Readlink stops using GetFinalPathNameByHandleW to try to resolve volumes to drive letters or shares, and instead converts the paths purely lexically to Win32 paths.
• syscall.Readlink and windows.Readlink either get updated to match the new os.Readlink behavior sans fixLongPath, or become deprecated only on Windows. (I don't have a preference as to which, but I'm not sure how to plumb a GODEBUG setting that far down in the standard library.)
  • syscall.Readlink does not currently match os.Readlink on Windows because it doesn't transform NT volume paths to Win32 paths.
  • syscall.Readlink does not currently match syscall.Readlink on POSIX platforms because POSIX does not treat mount points as symlinks.
  • syscall.Readlink should remain non-deprecated on POSIX platforms either way.

os.Lstat and os.Readlink stop treating mount points as symlinks, and instead report them as irregular files.

Although we end up not flaging mount points as symlinks, IMO it is still usefull that os.Readlink supports mount points (IO_REPARSE_TAG_MOUNT_POINT). Doing a quick web search, I can see many os.Readlink calls explicitly using it for mount points, and other languages do also support them in their "read link" equivalent:

• dotnet: https://github.com/dotnet/runtime/blob/18ee31dc1e265750b792ec1b1ee57794e4a87b04/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs#L637
• boost: https://github.com/boostorg/filesystem/blob/53eabaeabbf85fda2915a03612323df77fbaf23d/src/operations.cpp#L4138C7-L4138C7
• cpyhton: https://github.com/zooba/cpython/blob/941e81255e9e862838945c7e49d82d64e5bc9e64/Modules/posixmodule.c#L7859C52-L7859C52

About the nt to win32 path mapping, cpython has this gem: https://github.com/boostorg/filesystem/blob/53eabaeabbf85fda2915a03612323df77fbaf23d/src/operations.cpp#L2200.
Rust also does nt to win32 path mapping, but a little simpler:
https://github.com/rust-lang/rust/blob/5103173af1948430886534268d8f18b7e8386234/library/std/src/sys/windows/fs.rs#L542

Hmm, so we would report mount points as ModeIrregular (instead of ModeSymlink), but allow os.Readlink to resolve them anyway if it is called explicitly?

That does appear to be consistent with what cpython does, at least:
https://github.com/zooba/cpython/blob/67444902a0f10419a557d0a2d3b8675c31b075a9/Python/fileutils.c#L1098-L1104

But Boost does uniformly treat them as symlinks:
https://github.com/boostorg/filesystem/blob/53eabaeabbf85fda2915a03612323df77fbaf23d/src/windows_tools.hpp#L74-L89
(OTOH, I have to wonder if Boost also stumbles over mount points that refer to directories. The assumption that mount point == directory junction seems incorrect, given the evidence in #29746 (comment). 😅)

Doing a quick web search, I can see many os.Readlink calls explicitly using it for mount points

I'm having trouble finding any calls to os.Readlink that are specifically looking to resolve mount points. Can you give some examples (or a query to find them)?

I'm having trouble finding any calls to os.Readlink that are specifically looking to resolve mount points. Can you give some examples (or a query to find them)?

You can use this query: https://github.com/search?q=junction+os.readlink+language%3AGo&type=code&l=Go. Some selected examples: 1, 2, 3. I acknowledge that these are not popular projects and the value of the os.Readlink call is dubious, but my gut feeling is that there is value on keeping mount point support in os.Readlink (with the necessary fixes).

From the discussion of the current Python state, particularly python/cpython#82015 (comment), it's noted that when using a remote filesystem, symlinks and mount points are importantly different, as symlinks are resolved on the client (i.e. like POSIX) but the mount-point has to be resolved server-side when transitioning it, i.e. symlink-resolving code that reacts to seeing ModeSymlink during a walk and tries to canonicalise it may do the wrong thing when seeing a mount point e.g. a drive-letter reference would be referring to a drive on the server, but resolved to a drive on the client.

os.ReadLink being able to resolve any kind of name-surrogate is kind-of nice, but since it failed for volume mounts (the one place I'd tried to use it), I ended up using the Win32 API calls when I needed it anyway. At one point I had code that differentiated a mount point from symlink by checking that os.ReadLink returned its input. I would have changed that into winio.DecodeReparsePoint before merging, but we actually replaced the use of both symlinks and mount points with Bind Filter in later evolution of that PR, removing this distinction. Bindfilter may also be using reparse points (ProjFS does); but almost certainly not either of the tags we're talking about here, so it should just look like a directory/file unless calling low-level Windows-specific repase-point-supporting APIs.

So I have no strong feelings on os.ReadLink: it seems to me that in the Python equivalent of this discussion, it was kept able to handle both symlinks and mount/junctions simply because it already could and there wasn't a strong need seen to change that. So having it either able to read a mount point into its win32 path or fail with "That's not a symlink" seems fine to me, and the latter is consistent with Lstat not claiming it's a symlink either. (I don't think failing for volume mounts but succeeding for drive-letter file or directory junctions or similar is great... but since that's the current status quo, it's defensible.)

A few observations from sampling the first page of the query results in #63703 (comment).

There are lots of clones of https://github.com/rclone/rclone/. It only appears to call os.Readlink for links it has translated (but erroneously assumes that links are absolute).

It only translates links for actual symlinks, and actually goes out of its way not to translate mount points that way:
https://github.com/rclone/rclone/blob/0548e61910856bfd719e232f89448213e35cb4e6/backend/local/local.go#L563-L568

There are a couple of clones of https://github.com/BishopFox/sliver. It claims: “There is an edge case where if you are trying to download a junction on Windows, you will get access denied”, and it appears to be trying to work around that. It isn't at all clear to me what causes that error or how the attempted workaround works, but I have a suspicion that it may be the same sort of case as in #29746 (comment).

That workaround was added recently by @RafBishopFox, so perhaps they can shed some light on the problem?

There are a couple of clones of https://github.com/spicetify/spicetify-cli, but from what I can tell the calls to os.Readlink are in Linux-specific code, and the code that deals with Windows junctions appears to be unrelated.

Its remaining calls to filepath.EvalsSymlinks are in GetExecutableDir, and in that case it seems like they need to evaluate a file-symlink to the executable rather than a directory-junction. (The program uses GetExecutableDir to locate another subdirectory, and reading that subdirectory will succeed regardless of whether the returned directory is a mount point or its underlying target.)

https://github.com/buildkite/agent uses os.Readlink on some Linux-specific paths (in devfs and procfs). Its only portable use of os.Readlink is for a symlink created by os.Symlink:
https://github.com/buildkite/agent/blob/a4f1da625523e62d394f2a4a42d437c1a2c3e575/clicommand/agent_start.go#L1278-L1301

https://github.com/cloudfoundry/groot-windows does use os.Readlink, but as far as I can tell only on an actual symlink to a regular file. For junctions, it uses its own getSymlinkDest helper that uses syscall.CreateFile to open and convert the reparse buffer itself.

There are a couple of clones of https://github.com/vanadium-archive/go.jiri, but from what I can tell none of them have been updated since 2016 — and they appear to only use actual symlinks (created by os.Symlink) anyway. (And it looks like a false-positive in the GitHub search: it uses the word “conjunction”, not “junction”. 😵‍💫)

https://github.com/tranvictor/jarvis uses os.Readlink in various file-reading utilities, but appears to do so incorrectly: it assumes that the path is absolute (compare #57766), and it looks like the subsequent calls would work correctly on link paths anyway (so the calls to Readlink are actually count-productive 😅).

https://github.com/petethacker/ll uses os.Readlink, but appears to only use its result for logging. It's not clear to me what the tool is intended to do (its README is quite sparse and https://pkg.go.dev/github.com/petethacker/ll shows no package documentaion).

https://github.com/nyaosorg/go-windows-junction uses os.Readlink for logging in tests and examples. That would indeed break if os.Readlink were changed to no longer resolve mount points. This is the first unambiguous use of os.Readlink for mount points I've seen!

https://github.com/onozaty/go-sandbox also uses os.Readlink to explicitly read a junction target. From what I can tell, the code in that repo is essentially entirely examples, so it isn't clear to me whether its usage represents testing out a real use-case or just experimenting with the existing semantics.

Since the vast majority of calls to os.Readlink seem to be wrong anyway, I agree that it's probably reasonable to allow it to continue to read junction targets for the very few packages that do so intentionally and correctly (mainly https://github.com/nyaosorg/go-windows-junction).

But I do think it is important to arrange that we do not report junctions as symlinks and do not try to resolve them in filepath.EvalSymlinks. It seems clear to me that a lot of programs do use filepath.EvalSymlinks to try to resolve paths to something “canonical”, and those programs should generally resolve paths to a drive letter rather than a volume name that might not even be on the local machine.

Great summary @bcmills. To be clear, I'm completely onboarded on not reporting junctions as symlinks. I'm just doing some pushback to limit the blast radius of this change.

There are a couple of clones of https://github.com/BishopFox/sliver. It claims: “There is an edge case where if you are trying to download a junction on Windows, you will get access denied”, and it appears to be trying to work around that. It isn't at all clear to me what causes that error or how the attempted workaround works, but I have a suspicion that it may be the same sort of case as in #29746 (comment).

That workaround was added recently by @RafBishopFox, so perhaps they can shed some light on the problem?

I originally wrote that code sometime last year, so my recollection is a bit fuzzy. I was working on functionality in Sliver to create and send back an archive of a specified file or directory on a remote machine given filters (like *.docx to only bring back Word documents in a directory). In the case of a directory, I was getting access denied errors when trying to download files when the specified directory was a junction (reading the files from the directory pointed to by the junction did not result in an error). My workaround was to use os.Lstat to get information about the directory without resolving the junction. Then, if the resulting FileInfo object indicated that we were working with a symlink/junction, the code would resolve the junction and use the resolved path to read the files within the directory.

I cannot reproduce the behavior now, so maybe something else was going on with my test scenario. I remember it was an odd edge case, and for some reason, the junction was not being resolved automatically.

@RafBishopFox, I wonder if the problem you were running into was one of the ones fixed by https://go.dev/cl/463177. 🤔

@RafBishopFox, I wonder if the problem you were running into was one of the ones fixed by https://go.dev/cl/463177. 🤔

Quite possibly! At the time, I think I chalked it up to a quirk with how I was trying to do things, and since the workaround was easy to implement, I did not spend a ton of time investigating it.

What is the current proposal and is there general agreement for it yet?

I got here while trying to figure out why syscall.Readlink and golang.org/x/sys/windows.Readlink have slightly different behavior. The former returns the SubstituteName, while the latter the PrintName. The docs say that the former is the actual pathname, while the latter is for user consumption. Just wanted to point this out as another consideration in this issue.

The problem that got me here is trying to determine whether a path refers to a Volume Shadow Copy, which are identified by volume names, such as \\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy10. I can create a shadow copy of D:\, and then symlink it to C:\D. Currently, os.Readlink(`C:\D`) and filepath.EvalSymlinks(`C:\D`) return a file not found error.

To figure out whether a path like C:\D or C:\D\SomeFile refers to a shadow copy, I need to call GetFinalPathNameByHandle with the VOLUME_NAME_NT flag. Other flags return errors since a shadow copy does not have a volume GUID or a drive letter associated with it. Even with this solution, I found that passing the symlink itself (C:\D) to GetFinalPathNameByHandle just returns the same path rather than the device name, so now I'm doing a syscall.Readlink followed by GetFinalPathNameByHandle.

I think we are still waiting for @qmuntal to return from a break and weigh in here, or someone else from Microsoft to give an opinion.

I think we are still waiting for @qmuntal to return from a break and weigh in here

I'm back, so I'll have to weigh in 😄. As said before, we should do this.

Answering to @mxk:

I got here while trying to figure out why syscall.Readlink and golang.org/x/sys/windows.Readlink have slightly different behavior. The former returns the SubstituteName, while the latter the PrintName. The docs say that the former is the actual pathname, while the latter is for user consumption. Just wanted to point this out as another consideration in this issue.

We should always be using the SubstituteName name, not the PrintName. From the latter is just the display name of the reparse point and it can show something completely unrelated to the target (quote taken from dotnet/runtime). Could you file a separate issue for that?

Currently, os.Readlink(C:\D) and filepath.EvalSymlinks(C:\D) return a file not found error.

That issue will be solved if this proposal is approved, as neither os.Readlink nor filepath.EvalSymlinks will call GetFinalPathNameByHandle. On the other hand, you will still need to call GetFinalPathNameByHandle with the appropriate flags to know if a path refers to a shadow copy. I think this is find.

Have all remaining concerns about this proposal been addressed?

Proposal details are in #63703 (comment)

It is a little hard to figure out the exact consequences. I would recommend testing against something akin to the list that is in 40180 comment (I can help creating the insane directory on at least two networked computers) and I would suggest apart from whatever GODEBUG is to also adhere to the SymlinkEvaluation policy described here in any new code. Especially resolving drive letters on a different machine that exist locally is going to be a world of hurt all of a sudden once it no longer never happens on a magenta thursday for joe with the hat.

@ericwj none of this seems inconsistent with the proposal. Is there an API for Windows already that respects SymlinkEvaluation policy? It seems like os.Readlink should return an error when the link is disallowed by policy, and then EvalSymlinks would "just work". So is there some function os.Readlink should be using?

I am happy to see a well thought-through proposal. Tossing canonicalization leads to the solution it seems.

GetFinalPathNameByHandle yields for links on a remote machine

The symbolic link cannot be followed because its type is disabled. (0x800705B7)

Not using that API anymore might also scrub this HRESULT. If os.Readlink or filepath.EvalSymlinks resolve links to a remote machine without this HRESULT through the use of some variant of Stat then if you can explain how that works in terms of Win32 API sequences I can help think towards a solution. I have quite proudly still less than some 20 lines of go written in my life and I gave up after failing trying to view the whole file in the PR on the other source control server website.

So like I said, add tests against a remote machine. Perhaps more than just that from the list I tested years ago. Proof the pudding.

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

Proposal details are in #63703 (comment)

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

Proposal details are in #63703 (comment)

Change https://go.dev/cl/565136 mentions this issue: os: don't treat mount points as symbolic links

Change https://go.dev/cl/567735 mentions this issue: os: don't normalize volumes to drive letters in os.Readlink

Change https://go.dev/cl/569195 mentions this issue: os: fix 63703.md release notes