]> Cypherpunks repositories - gostls13.git/commitdiff
Revert "cmd/go: use os.Rename to move files on Windows"
authorQuim Muntal <quimmuntal@gmail.com>
Tue, 9 Sep 2025 16:02:28 +0000 (09:02 -0700)
committerGopher Robot <gobot@golang.org>
Tue, 9 Sep 2025 18:02:22 +0000 (11:02 -0700)
This reverts CL 691255.

Reason for revert: SetNamedSecurityInfo sometimes fails when the path contains symbolic links.

Fixes #74864

Change-Id: Iaf1a5692b35d58c523fd513f27bad9a2e7a334cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/702155
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
src/cmd/go/internal/work/shell.go
src/cmd/go/internal/work/shell_nonwindows.go [deleted file]
src/cmd/go/internal/work/shell_windows.go [deleted file]
src/internal/syscall/windows/security_windows.go
src/internal/syscall/windows/types_windows.go
src/internal/syscall/windows/zsyscall_windows.go

index e75b1c33fc5a2547652379bd568ce35c7846ab36..284ed26f223d7d32a304651976df039841e7ab33 100644 (file)
@@ -132,11 +132,47 @@ func (sh *Shell) moveOrCopyFile(dst, src string, perm fs.FileMode, force bool) e
                return sh.CopyFile(dst, src, perm, force)
        }
 
-       if err := sh.move(src, dst, perm); err == nil {
-               if cfg.BuildX {
-                       sh.ShowCmd("", "mv %s %s", src, dst)
+       // On Windows, always copy the file, so that we respect the NTFS
+       // permissions of the parent folder. https://golang.org/issue/22343.
+       // What matters here is not cfg.Goos (the system we are building
+       // for) but runtime.GOOS (the system we are building on).
+       if runtime.GOOS == "windows" {
+               return sh.CopyFile(dst, src, perm, force)
+       }
+
+       // If the destination directory has the group sticky bit set,
+       // we have to copy the file to retain the correct permissions.
+       // https://golang.org/issue/18878
+       if fi, err := os.Stat(filepath.Dir(dst)); err == nil {
+               if fi.IsDir() && (fi.Mode()&fs.ModeSetgid) != 0 {
+                       return sh.CopyFile(dst, src, perm, force)
+               }
+       }
+
+       // The perm argument is meant to be adjusted according to umask,
+       // but we don't know what the umask is.
+       // Create a dummy file to find out.
+       // This avoids build tags and works even on systems like Plan 9
+       // where the file mask computation incorporates other information.
+       mode := perm
+       f, err := os.OpenFile(filepath.Clean(dst)+"-go-tmp-umask", os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
+       if err == nil {
+               fi, err := f.Stat()
+               if err == nil {
+                       mode = fi.Mode() & 0777
+               }
+               name := f.Name()
+               f.Close()
+               os.Remove(name)
+       }
+
+       if err := os.Chmod(src, mode); err == nil {
+               if err := os.Rename(src, dst); err == nil {
+                       if cfg.BuildX {
+                               sh.ShowCmd("", "mv %s %s", src, dst)
+                       }
+                       return nil
                }
-               return nil
        }
 
        return sh.CopyFile(dst, src, perm, force)
diff --git a/src/cmd/go/internal/work/shell_nonwindows.go b/src/cmd/go/internal/work/shell_nonwindows.go
deleted file mode 100644 (file)
index c517fbe..0000000
+++ /dev/null
@@ -1,49 +0,0 @@
-// Copyright 2025 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build !windows
-
-package work
-
-import (
-       "errors"
-       "io/fs"
-       "os"
-       "path/filepath"
-)
-
-// move moves a file from src to dst setting the permissions
-// on the destination file to inherit the permissions from the
-// destination parent directory.
-func (sh *Shell) move(src, dst string, perm fs.FileMode) error {
-       // If the destination directory has the group sticky bit set,
-       // we have to copy the file to retain the correct permissions.
-       // https://golang.org/issue/18878
-       if fi, err := os.Stat(filepath.Dir(dst)); err == nil {
-               if fi.IsDir() && (fi.Mode()&fs.ModeSetgid) != 0 {
-                       return errors.ErrUnsupported
-               }
-       }
-       // The perm argument is meant to be adjusted according to umask,
-       // but we don't know what the umask is.
-       // Create a dummy file to find out.
-       // This works even on systems like Plan 9 where the
-       // file mask computation incorporates other information.
-       mode := perm
-       f, err := os.OpenFile(filepath.Clean(dst)+"-go-tmp-umask", os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm)
-       if err == nil {
-               fi, err := f.Stat()
-               if err == nil {
-                       mode = fi.Mode() & 0777
-               }
-               name := f.Name()
-               f.Close()
-               os.Remove(name)
-       }
-
-       if err := os.Chmod(src, mode); err != nil {
-               return err
-       }
-       return os.Rename(src, dst)
-}
diff --git a/src/cmd/go/internal/work/shell_windows.go b/src/cmd/go/internal/work/shell_windows.go
deleted file mode 100644 (file)
index 9b80eab..0000000
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright 2025 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package work
-
-import (
-       "internal/syscall/windows"
-       "io/fs"
-       "os"
-       "unsafe"
-)
-
-// move moves a file from src to dst, setting the security information
-// on the destination file to inherit the permissions from the
-// destination parent directory.
-func (sh *Shell) move(src, dst string, perm fs.FileMode) (err error) {
-       if err := os.Rename(src, dst); err != nil {
-               return err
-       }
-       defer func() {
-               if err != nil {
-                       os.Remove(dst) // clean up if we failed to set the mode or security info
-               }
-       }()
-       if err := os.Chmod(dst, perm); err != nil {
-               return err
-       }
-       // We need to respect the ACL permissions of the destination parent folder.
-       // https://go.dev/issue/22343.
-       var acl windows.ACL
-       if err := windows.InitializeAcl(&acl, uint32(unsafe.Sizeof(acl)), windows.ACL_REVISION); err != nil {
-               return err
-       }
-       secInfo := windows.DACL_SECURITY_INFORMATION | windows.UNPROTECTED_DACL_SECURITY_INFORMATION
-       return windows.SetNamedSecurityInfo(dst, windows.SE_FILE_OBJECT, secInfo, nil, nil, &acl, nil)
-}
index 2c35ad31f4d039f0a6ae8aa98079c2d1db2f5c41..f0ab52ac819baf74d29698ff76671c34d648f754 100644 (file)
@@ -262,6 +262,3 @@ func GetSidSubAuthorityCount(sid *syscall.SID) uint8 {
        defer runtime.KeepAlive(sid)
        return *(*uint8)(unsafe.Pointer(getSidSubAuthorityCount(sid)))
 }
-
-//sys  InitializeAcl(acl *ACL, length uint32, revision uint32) (err error) = advapi32.InitializeAcl
-//sys  SetNamedSecurityInfo(objectName string, objectType SE_OBJECT_TYPE, securityInformation SECURITY_INFORMATION, owner *syscall.SID, group *syscall.SID, dacl *ACL, sacl *ACL) (ret error) = advapi32.SetNamedSecurityInfoW
index cb0a48f0802c7ab9ed6f42e6b4a0a8a5df8824fc..33767391434a32fd603796b6fb7ab56951e1aa9f 100644 (file)
@@ -276,21 +276,3 @@ type FILE_COMPLETION_INFORMATION struct {
 // https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-osversioninfoexa
 // https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_osversioninfoexw
 const VER_NT_WORKSTATION = 0x0000001
-
-// https://learn.microsoft.com/en-us/windows/win32/api/accctrl/ne-accctrl-se_object_type
-type SE_OBJECT_TYPE uint32
-
-const (
-       SE_UNKNOWN_OBJECT_TYPE SE_OBJECT_TYPE = 0
-       SE_FILE_OBJECT         SE_OBJECT_TYPE = 1
-)
-
-// https://learn.microsoft.com/en-us/windows/win32/secauthz/security-information
-type SECURITY_INFORMATION uint32
-
-const (
-       DACL_SECURITY_INFORMATION             SECURITY_INFORMATION = 0x00000004
-       UNPROTECTED_DACL_SECURITY_INFORMATION SECURITY_INFORMATION = 0x20000000
-)
-
-const ACL_REVISION = 2
index c08b2ccdbab3d0e671d62be7545f38e5d7b0e9d4..90cf0b92a49bc44b8296929d2248f66dac206f40 100644 (file)
@@ -54,7 +54,6 @@ var (
        procGetSidSubAuthorityCount           = modadvapi32.NewProc("GetSidSubAuthorityCount")
        procImpersonateLoggedOnUser           = modadvapi32.NewProc("ImpersonateLoggedOnUser")
        procImpersonateSelf                   = modadvapi32.NewProc("ImpersonateSelf")
-       procInitializeAcl                     = modadvapi32.NewProc("InitializeAcl")
        procIsValidSid                        = modadvapi32.NewProc("IsValidSid")
        procLogonUserW                        = modadvapi32.NewProc("LogonUserW")
        procLookupPrivilegeValueW             = modadvapi32.NewProc("LookupPrivilegeValueW")
@@ -63,7 +62,6 @@ var (
        procOpenThreadToken                   = modadvapi32.NewProc("OpenThreadToken")
        procQueryServiceStatus                = modadvapi32.NewProc("QueryServiceStatus")
        procRevertToSelf                      = modadvapi32.NewProc("RevertToSelf")
-       procSetNamedSecurityInfoW             = modadvapi32.NewProc("SetNamedSecurityInfoW")
        procSetTokenInformation               = modadvapi32.NewProc("SetTokenInformation")
        procProcessPrng                       = modbcryptprimitives.NewProc("ProcessPrng")
        procGetAdaptersAddresses              = modiphlpapi.NewProc("GetAdaptersAddresses")
@@ -168,14 +166,6 @@ func ImpersonateSelf(impersonationlevel uint32) (err error) {
        return
 }
 
-func InitializeAcl(acl *ACL, length uint32, revision uint32) (err error) {
-       r1, _, e1 := syscall.Syscall(procInitializeAcl.Addr(), 3, uintptr(unsafe.Pointer(acl)), uintptr(length), uintptr(revision))
-       if r1 == 0 {
-               err = errnoErr(e1)
-       }
-       return
-}
-
 func IsValidSid(sid *syscall.SID) (valid bool) {
        r0, _, _ := syscall.Syscall(procIsValidSid.Addr(), 1, uintptr(unsafe.Pointer(sid)), 0, 0)
        valid = r0 != 0
@@ -244,23 +234,6 @@ func RevertToSelf() (err error) {
        return
 }
 
-func SetNamedSecurityInfo(objectName string, objectType SE_OBJECT_TYPE, securityInformation SECURITY_INFORMATION, owner *syscall.SID, group *syscall.SID, dacl *ACL, sacl *ACL) (ret error) {
-       var _p0 *uint16
-       _p0, ret = syscall.UTF16PtrFromString(objectName)
-       if ret != nil {
-               return
-       }
-       return _SetNamedSecurityInfo(_p0, objectType, securityInformation, owner, group, dacl, sacl)
-}
-
-func _SetNamedSecurityInfo(objectName *uint16, objectType SE_OBJECT_TYPE, securityInformation SECURITY_INFORMATION, owner *syscall.SID, group *syscall.SID, dacl *ACL, sacl *ACL) (ret error) {
-       r0, _, _ := syscall.Syscall9(procSetNamedSecurityInfoW.Addr(), 7, uintptr(unsafe.Pointer(objectName)), uintptr(objectType), uintptr(securityInformation), uintptr(unsafe.Pointer(owner)), uintptr(unsafe.Pointer(group)), uintptr(unsafe.Pointer(dacl)), uintptr(unsafe.Pointer(sacl)), 0, 0)
-       if r0 != 0 {
-               ret = syscall.Errno(r0)
-       }
-       return
-}
-
 func SetTokenInformation(tokenHandle syscall.Token, tokenInformationClass uint32, tokenInformation unsafe.Pointer, tokenInformationLength uint32) (err error) {
        r1, _, e1 := syscall.Syscall6(procSetTokenInformation.Addr(), 4, uintptr(tokenHandle), uintptr(tokenInformationClass), uintptr(tokenInformation), uintptr(tokenInformationLength), 0, 0)
        if r1 == 0 {