From: Quim Muntal Date: Tue, 9 Sep 2025 16:02:28 +0000 (-0700) Subject: Revert "cmd/go: use os.Rename to move files on Windows" X-Git-Tag: go1.26rc1~936 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=3b3b16957cd466baef3af92383de324230ad993c;p=gostls13.git Revert "cmd/go: use os.Rename to move files on Windows" 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 Auto-Submit: Quim Muntal Reviewed-by: Cherry Mui Reviewed-by: Damien Neil --- diff --git a/src/cmd/go/internal/work/shell.go b/src/cmd/go/internal/work/shell.go index e75b1c33fc..284ed26f22 100644 --- a/src/cmd/go/internal/work/shell.go +++ b/src/cmd/go/internal/work/shell.go @@ -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 index c517fbe196..0000000000 --- a/src/cmd/go/internal/work/shell_nonwindows.go +++ /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 index 9b80eab07f..0000000000 --- a/src/cmd/go/internal/work/shell_windows.go +++ /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) -} diff --git a/src/internal/syscall/windows/security_windows.go b/src/internal/syscall/windows/security_windows.go index 2c35ad31f4..f0ab52ac81 100644 --- a/src/internal/syscall/windows/security_windows.go +++ b/src/internal/syscall/windows/security_windows.go @@ -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 diff --git a/src/internal/syscall/windows/types_windows.go b/src/internal/syscall/windows/types_windows.go index cb0a48f080..3376739143 100644 --- a/src/internal/syscall/windows/types_windows.go +++ b/src/internal/syscall/windows/types_windows.go @@ -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 diff --git a/src/internal/syscall/windows/zsyscall_windows.go b/src/internal/syscall/windows/zsyscall_windows.go index c08b2ccdba..90cf0b92a4 100644 --- a/src/internal/syscall/windows/zsyscall_windows.go +++ b/src/internal/syscall/windows/zsyscall_windows.go @@ -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 {