cmd/go: go fmt aborts with operation not permitted when using mounted vmware shared folders (hgfs)

[mdepot]

What version of Go are you using (go version)?

$ go version
go version go1.20.4 linux/amd64

Does this issue reproduce with the latest release?

Yes, version 1.20.4 is the latest release as of this writing

What operating system and processor architecture are you using (go env)?

Rocky Linux 8.7 (amd64) in a VM, running on top of VMware Workstation 17 Pro, on Windows 10

go env:

$ go env
GOARCH="amd64"
GOCACHE="/home/myuser/.cache/go-build"
GOENV="/home/myuser/.config/go/env"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOMODCACHE="/home/myuser/go/pkg/mod"
GOOS="linux"
GOPATH="/home/myuser/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVERSION="go1.20.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/mnt/hgfs/vmshare/golang/myapp/go.mod"
CGO_CFLAGS="-O2 -g"
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2445446007=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ ll
total 52
drwxrwxrwx. 1 root root  4096 May 16 08:56 cmd
-rwxrwxrwx. 1 root root   840 May 15 16:01 go.mod
-rwxrwxrwx. 1 root root 47337 May 15 16:01 go.sum
-rwxrwxrwx. 1 root root     0 May 15 16:01 LICENSE
-rwxrwxrwx. 1 root root   152 May 15 16:01 main.go

$ sudo chown myuser *.go
[sudo] password for myuser: 

$ sudo chmod 640 *.go

$ ll
total 52
drwxrwxrwx. 1 root root  4096 May 16 08:56 cmd
-rwxrwxrwx. 1 root root   840 May 15 16:01 go.mod
-rwxrwxrwx. 1 root root 47337 May 15 16:01 go.sum
-rwxrwxrwx. 1 root root     0 May 15 16:01 LICENSE
-rwxrwxrwx. 1 root root   152 May 15 16:01 main.go

$ go fmt main.go
main.go
chmod ./main.go.2356257763: operation not permitted
exit status 2

Comments

The go fmt command above appears to create a temporary file in the current dir, and then apparently runs chmod on
that file. It would seem the chmod should not be needed to gain access to the file by go fmt, so I presume the
purpose of the chmod was actually to impose access restriction to the temp file by other users/processes during the
formatting operation.

In my case the file system of the working dir is a mounted vmware shared folder. This is a windows directory that
is shared with the linux VM by means of vmware tools. From the perspective of linux, the file permissions on such a
mount will always be 777 and owned by root, as demonstrated by the ineffective chown and chmod commands above. When
the go fmt command tries and fails to do a chmod on the temp file, it completely aborts the code formatting
operation.

Handling for this should be improved so that code formatting can still work when using various (less capable)
mounted file system types. It may make sense to check the permissions of the temp file, and only call chmod if the
permissions on the temp file are actually less restrictive than the permissions on source file for example. Another
option might be to attempt the chmod but not completely abort on failure, although the potential security
implications of that option should be considered.

[heschi]

cc @griesemer @mvdan

[ianlancetaylor]

The chmod isn't to protect the file. It's simply that go fmt creates a backup file before rewriting the original file. It creates the backup file by creating a temporary file and then calling chmod to set the permissions of the temporary file to the permissions of the original file. Then if rewriting the original file fails, it renames the backup file to the original file, so that the file is not lost.

It would be slightly more efficient and slightly more secure to create the backup file with the correct permissions in the first place, but the code just calls os.CreateTemp which doesn't have a way to specify the permissions. If we did that, somehow, then I assume that this problem would go away. We could also skip the chmod if the permissions happen to be exactly the same as the temporary file (0o600) but that is not particularly likely and is not the case in your example.

[gopherbot]

Change https://go.dev/cl/495316 mentions this issue: cmd/gofmt: don't clobber original file on permission error

[gopherbot]

Change https://go.dev/cl/496155 mentions this issue: cmd/gofmt: fix a data race in TestPermissions