os: don't treat mount points as symbolic links

This CL changes the behavior of os.Lstat to stop setting the
os.ModeSymlink type mode bit for mount points on Windows. As a result,
filepath.EvalSymlinks no longer evaluates mount points, which was the
cause of many inconsistencies and bugs.

Additionally, os.Lstat starts setting the os.ModeIrregular type mode bit
for all reparse tags on Windows, except for those that are explicitly
supported by the os package, which, since this CL, doesn't include mount
points. This helps to identify files that need special handling outside
of the os package.

This behavior is controlled by the `winsymlink` GODEBUG setting.
For Go 1.23, it defaults to `winsymlink=1`.
Previous versions default to `winsymlink=0`.

Fixes #39786
Fixes #40176
Fixes #61893
Updates #63703
Updates #40180
Updates #63429

Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-windows-arm64
Change-Id: I2e7372ab8862f5062667d30db6958d972bce5407
Reviewed-on: https://go-review.googlesource.com/c/go/+/565136
Reviewed-by: Bryan Mills <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>

Author: qmuntal

doc/godebug.md

@@ -126,6 +126,19 @@ for example,
 see the [runtime documentation](/pkg/runtime#hdr-Environment_Variables)
 and the [go command documentation](/cmd/go#hdr-Build_and_test_caching).
 
+### Go 1.23
+
+Go 1.23 changed the mode bits reported by [`os.Lstat`](/pkg/os#Lstat) and [`os.Stat`](/pkg/os#Stat)
+for reparse points, which can be controlled with the `winsymlink` setting.
+As of Go 1.23 (`winsymlink=1`), mount points no longer have [`os.ModeSymlink`](/pkg/os#ModeSymlink)
+set, and reparse points that are not symlinks, Unix sockets, or dedup files now
+always have [`os.ModeIrregular`](/pkg/os#ModeIrregular) set. As a result of these changes,
+[`filepath.EvalSymlinks`](/pkg/path/filepath#EvalSymlinks) no longer evaluates
+mount points, which was a source of many inconsistencies and bugs.
+At previous versions (`winsymlink=0`), mount points are treated as symlinks,
+and other reparse points with non-default [`os.ModeType`](/pkg/os#ModeType) bits
+(such as [`os.ModeDir`](/pkg/os#ModeDir)) do not have the `ModeIrregular` bit set.
+
 ### Go 1.22
 
 Go 1.22 adds a configurable limit to control the maximum acceptable RSA key size

doc/next/6-stdlib/99-minor/os/61893.md

@@ -0,0 +1,7 @@
+On Windows, the mode bits reported by [`os.Lstat`](/pkg/os#Lstat) and [`os.Stat`](/pkg/os#Stat)
+for reparse points changed. Mount points no longer have [`os.ModeSymlink`](/pkg/os#ModeSymlink) set,
+and reparse points that are not symlinks, Unix sockets, or dedup files now
+always have [`os.ModeIrregular`](/pkg/os#ModeIrregular) set.
+This behavior is controlled by the `winsymlink` setting.
+For Go 1.23, it defaults to `winsymlink=1`.
+Previous versions default to `winsymlink=0`.

doc/next/6-stdlib/99-minor/path/filepath/63703.md

@@ -0,0 +1,5 @@
+On Windows, [`filepath.EvalSymlinks`](/pkg/path/filepath#EvalSymlinks) no longer evaluates
+mount points, which was a source of many inconsistencies and bugs.
+This behavior is controlled by the `winsymlink` setting.
+For Go 1.23, it defaults to `winsymlink=1`.
+Previous versions default to `winsymlink=0`.

src/internal/godebugs/table.go

@@ -49,6 +49,7 @@ var All = []Info{
 	{Name: "tlsmaxrsasize", Package: "crypto/tls"},
 	{Name: "tlsrsakex", Package: "crypto/tls", Changed: 22, Old: "1"},
 	{Name: "tlsunsafeekm", Package: "crypto/tls", Changed: 22, Old: "1"},
+	{Name: "winsymlink", Package: "os", Changed: 22, Old: "0"},
 	{Name: "x509sha1", Package: "crypto/x509"},
 	{Name: "x509usefallbackroots", Package: "crypto/x509"},
 	{Name: "x509usepolicies", Package: "crypto/x509"},

src/os/os_windows_test.go

@@ -7,6 +7,7 @@ package os_test
 import (
 	"errors"
 	"fmt"
+	"internal/godebug"
 	"internal/poll"
 	"internal/syscall/windows"
 	"internal/syscall/windows/registry"
@@ -27,6 +28,8 @@ import (
 	"unsafe"
 )
 
+var winsymlink = godebug.New("winsymlink")
+
 // For TestRawConnReadWrite.
 type syscallDescriptor = syscall.Handle
 
@@ -90,9 +93,10 @@ func TestSameWindowsFile(t *testing.T) {
 }
 
 type dirLinkTest struct {
-	name    string
-	mklink  func(link, target string) error
-	issueNo int // correspondent issue number (for broken tests)
+	name         string
+	mklink       func(link, target string) error
+	issueNo      int // correspondent issue number (for broken tests)
+	isMountPoint bool
 }
 
 func testDirLinks(t *testing.T, tests []dirLinkTest) {
@@ -140,8 +144,8 @@ func testDirLinks(t *testing.T, tests []dirLinkTest) {
 			t.Errorf("failed to stat link %v: %v", link, err)
 			continue
 		}
-		if !fi1.IsDir() {
-			t.Errorf("%q should be a directory", link)
+		if tp := fi1.Mode().Type(); tp != fs.ModeDir {
+			t.Errorf("Stat(%q) is type %v; want %v", link, tp, fs.ModeDir)
 			continue
 		}
 		if fi1.Name() != filepath.Base(link) {
@@ -158,13 +162,16 @@ func testDirLinks(t *testing.T, tests []dirLinkTest) {
 			t.Errorf("failed to lstat link %v: %v", link, err)
 			continue
 		}
-		if m := fi2.Mode(); m&fs.ModeSymlink == 0 {
-			t.Errorf("%q should be a link, but is not (mode=0x%x)", link, uint32(m))
-			continue
+		var wantType fs.FileMode
+		if test.isMountPoint && winsymlink.Value() != "0" {
+			// Mount points are reparse points, and we no longer treat them as symlinks.
+			wantType = fs.ModeIrregular
+		} else {
+			// This is either a real symlink, or a mount point treated as a symlink.
+			wantType = fs.ModeSymlink
 		}
-		if m := fi2.Mode(); m&fs.ModeDir != 0 {
-			t.Errorf("%q should be a link, not a directory (mode=0x%x)", link, uint32(m))
-			continue
+		if tp := fi2.Mode().Type(); tp != wantType {
+			t.Errorf("Lstat(%q) is type %v; want %v", link, tp, fs.ModeDir)
 		}
 	}
 }
@@ -272,7 +279,8 @@ func TestDirectoryJunction(t *testing.T) {
 	var tests = []dirLinkTest{
 		{
 			// Create link similar to what mklink does, by inserting \??\ at the front of absolute target.
-			name: "standard",
+			name:         "standard",
+			isMountPoint: true,
 			mklink: func(link, target string) error {
 				var t reparseData
 				t.addSubstituteName(`\??\` + target)
@@ -282,7 +290,8 @@ func TestDirectoryJunction(t *testing.T) {
 		},
 		{
 			// Do as junction utility https://learn.microsoft.com/en-us/sysinternals/downloads/junction does - set PrintNameLength to 0.
-			name: "have_blank_print_name",
+			name:         "have_blank_print_name",
+			isMountPoint: true,
 			mklink: func(link, target string) error {
 				var t reparseData
 				t.addSubstituteName(`\??\` + target)
@@ -296,7 +305,8 @@ func TestDirectoryJunction(t *testing.T) {
 	if mklinkSupportsJunctionLinks {
 		tests = append(tests,
 			dirLinkTest{
-				name: "use_mklink_cmd",
+				name:         "use_mklink_cmd",
+				isMountPoint: true,
 				mklink: func(link, target string) error {
 					output, err := testenv.Command(t, "cmd", "/c", "mklink", "/J", link, target).CombinedOutput()
 					if err != nil {
@@ -1414,16 +1424,10 @@ func TestAppExecLinkStat(t *testing.T) {
 	if lfi.Name() != pythonExeName {
 		t.Errorf("Stat %s: got %q, but wanted %q", pythonPath, lfi.Name(), pythonExeName)
 	}
-	if m := lfi.Mode(); m&fs.ModeSymlink != 0 {
-		t.Errorf("%q should be a file, not a link (mode=0x%x)", pythonPath, uint32(m))
-	}
-	if m := lfi.Mode(); m&fs.ModeDir != 0 {
-		t.Errorf("%q should be a file, not a directory (mode=0x%x)", pythonPath, uint32(m))
-	}
-	if m := lfi.Mode(); m&fs.ModeIrregular == 0 {
+	if tp := lfi.Mode().Type(); tp != fs.ModeIrregular {
 		// A reparse point is not a regular file, but we don't have a more appropriate
 		// ModeType bit for it, so it should be marked as irregular.
-		t.Errorf("%q should not be a regular file (mode=0x%x)", pythonPath, uint32(m))
+		t.Errorf("%q should not be a an irregular file (mode=0x%x)", pythonPath, uint32(tp))
 	}
 
 	if sfi.Name() != pythonExeName {

src/os/types_windows.go

@@ -5,6 +5,7 @@
 package os
 
 import (
+	"internal/godebug"
 	"internal/syscall/windows"
 	"sync"
 	"syscall"
@@ -151,37 +152,86 @@ func newFileStatFromWin32finddata(d *syscall.Win32finddata) *fileStat {
 // and https://learn.microsoft.com/en-us/windows/win32/fileio/reparse-point-tags.
 func (fs *fileStat) isReparseTagNameSurrogate() bool {
 	// True for IO_REPARSE_TAG_SYMLINK and IO_REPARSE_TAG_MOUNT_POINT.
-	return fs.ReparseTag&0x20000000 != 0
-}
-
-func (fs *fileStat) isSymlink() bool {
-	// As of https://go.dev/cl/86556, we treat MOUNT_POINT reparse points as
-	// symlinks because otherwise certain directory junction tests in the
-	// path/filepath package would fail.
-	//
-	// However,
-	// https://learn.microsoft.com/en-us/windows/win32/fileio/hard-links-and-junctions
-	// seems to suggest that directory junctions should be treated like hard
-	// links, not symlinks.
-	//
-	// TODO(bcmills): Get more input from Microsoft on what the behavior ought to
-	// be for MOUNT_POINT reparse points.
-
-	return fs.ReparseTag == syscall.IO_REPARSE_TAG_SYMLINK ||
-		fs.ReparseTag == windows.IO_REPARSE_TAG_MOUNT_POINT
+	return fs.FileAttributes&syscall.FILE_ATTRIBUTE_REPARSE_POINT != 0 && fs.ReparseTag&0x20000000 != 0
 }
 
 func (fs *fileStat) Size() int64 {
 	return int64(fs.FileSizeHigh)<<32 + int64(fs.FileSizeLow)
 }
 
+var winsymlink = godebug.New("winsymlink")
+
 func (fs *fileStat) Mode() (m FileMode) {
+	if winsymlink.Value() == "0" {
+		return fs.modePreGo1_23()
+	}
+	if fs.FileAttributes&syscall.FILE_ATTRIBUTE_READONLY != 0 {
+		m |= 0444
+	} else {
+		m |= 0666
+	}
+
+	// Windows reports the FILE_ATTRIBUTE_DIRECTORY bit for reparse points
+	// that refer to directories, such as symlinks and mount points.
+	// However, we follow symlink POSIX semantics and do not set the mode bits.
+	// This allows users to walk directories without following links
+	// by just calling "fi, err := os.Lstat(name); err == nil && fi.IsDir()".
+	// Note that POSIX only defines the semantics for symlinks, not for
+	// mount points or other surrogate reparse points, but we treat them
+	// the same way for consistency. Also, mount points can contain infinite
+	// loops, so it is not safe to walk them without special handling.
+	if !fs.isReparseTagNameSurrogate() {
+		if fs.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
+			m |= ModeDir | 0111
+		}
+
+		switch fs.filetype {
+		case syscall.FILE_TYPE_PIPE:
+			m |= ModeNamedPipe
+		case syscall.FILE_TYPE_CHAR:
+			m |= ModeDevice | ModeCharDevice
+		}
+	}
+
+	if fs.FileAttributes&syscall.FILE_ATTRIBUTE_REPARSE_POINT != 0 {
+		switch fs.ReparseTag {
+		case syscall.IO_REPARSE_TAG_SYMLINK:
+			m |= ModeSymlink
+		case windows.IO_REPARSE_TAG_AF_UNIX:
+			m |= ModeSocket
+		case windows.IO_REPARSE_TAG_DEDUP:
+			// If the Data Deduplication service is enabled on Windows Server, its
+			// Optimization job may convert regular files to IO_REPARSE_TAG_DEDUP
+			// whenever that job runs.
+			//
+			// However, DEDUP reparse points remain similar in most respects to
+			// regular files: they continue to support random-access reads and writes
+			// of persistent data, and they shouldn't add unexpected latency or
+			// unavailability in the way that a network filesystem might.
+			//
+			// Go programs may use ModeIrregular to filter out unusual files (such as
+			// raw device files on Linux, POSIX FIFO special files, and so on), so
+			// to avoid files changing unpredictably from regular to irregular we will
+			// consider DEDUP files to be close enough to regular to treat as such.
+		default:
+			m |= ModeIrregular
+		}
+	}
+	return
+}
+
+// modePreGo1_23 returns the FileMode for the fileStat, using the pre-Go 1.23
+// logic for determining the file mode.
+// The logic is subtle and not well-documented, so it is better to keep it
+// separate from the new logic.
+func (fs *fileStat) modePreGo1_23() (m FileMode) {
 	if fs.FileAttributes&syscall.FILE_ATTRIBUTE_READONLY != 0 {
 		m |= 0444
 	} else {
 		m |= 0666
 	}
-	if fs.isSymlink() {
+	if fs.ReparseTag == syscall.IO_REPARSE_TAG_SYMLINK ||
+		fs.ReparseTag == windows.IO_REPARSE_TAG_MOUNT_POINT {
 		return m | ModeSymlink
 	}
 	if fs.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
@@ -199,19 +249,7 @@ func (fs *fileStat) Mode() (m FileMode) {
 		}
 		if m&ModeType == 0 {
 			if fs.ReparseTag == windows.IO_REPARSE_TAG_DEDUP {
-				// If the Data Deduplication service is enabled on Windows Server, its
-				// Optimization job may convert regular files to IO_REPARSE_TAG_DEDUP
-				// whenever that job runs.
-				//
-				// However, DEDUP reparse points remain similar in most respects to
-				// regular files: they continue to support random-access reads and writes
-				// of persistent data, and they shouldn't add unexpected latency or
-				// unavailability in the way that a network filesystem might.
-				//
-				// Go programs may use ModeIrregular to filter out unusual files (such as
-				// raw device files on Linux, POSIX FIFO special files, and so on), so
-				// to avoid files changing unpredictably from regular to irregular we will
-				// consider DEDUP files to be close enough to regular to treat as such.
+				// See comment in fs.Mode.
 			} else {
 				m |= ModeIrregular
 			}

src/path/filepath/path_test.go

@@ -1980,3 +1980,16 @@ func TestEscaping(t *testing.T) {
 		}
 	}
 }
+
+func TestEvalSymlinksTooManyLinks(t *testing.T) {
+	testenv.MustHaveSymlink(t)
+	dir := filepath.Join(t.TempDir(), "dir")
+	err := os.Symlink(dir, dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = filepath.EvalSymlinks(dir)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+}